search

ziatdinovmax / gpax

Gaussian Processes for Experimental Sciences
http://gpax.rtfd.io
MIT License
205 stars 27 forks source link

Suggestion: refactor acquisition.py to a class structure #42

Closed matthewcarbone closed 1 year ago

matthewcarbone commented 1 year ago

Currently, every acquisition function in acquisition.py is a function. I think these objects will function better as classes since they take a lot of common arguments and also share a common abstraction. In addition, it will make the campaigning in #33 much more straightforward. For example, while certain acquisition functions require different things (EI requires best_f whereas UCB requires beta), they could all have a common function, maybe update_as_function_of_data_and_observations that calculates best_f for EI and does nothing for UCB, allowing for extension to more complex acquisition functions later.

I will implement this as a backwards-compatible feature for #33 but I definitely think you should consider making this change!

ziatdinovmax commented 1 year ago

Yes, I was considering doing this but didn't have the bandwidth. One thing we should discuss is how we want to expand current acquisition functions and why refactoring will help in this expansion. I would generally not recommend doing refactoring just for the sake of refactoring.

matthewcarbone commented 1 year ago

@ziatdinovmax yes definitely. I am happy to help with this as I think I need to do it on the way to the campaigning.

One thing we should discuss is how we want to expand current acquisition functions and why refactoring will help in this expansion.

I think the point here is to provide a common abstraction. Definitely agree that refactoring for the sake of it is a bad idea for a deployed code (unless the refactoring will seriously help in development and won't break things).

Not only will this allow one to work more seamlessly with multiple acquisition functions at the same time (as in campaigning), it can in principle give users more of a guide to develop their own acquisition functions (by e.g. inheriting some abstract base class).