Suggestion: refactor acquisition.py to a class structure #55
Replies: 2 comments
-
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. |
Beta Was this translation helpful? Give feedback.
-
@ziatdinovmax yes definitely. I am happy to help with this as I think I need to do it on the way to the campaigning.
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). |
Beta Was this translation helpful? Give feedback.
-
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 requiresbest_f
whereas UCB requiresbeta
), they could all have a common function, maybeupdate_as_function_of_data_and_observations
that calculatesbest_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!
Beta Was this translation helpful? Give feedback.
All reactions