Update acquisition cost implementation

[uremes]

Summary:

This update removes additive_cost from the LCBSC acquisition function and adds an option to extend any acquisition function with an additive or multiplicative term.

Motivation: LCBSC was extended with an option to use additive_cost so that BOLFIRE could model the log-likelihood ratio but calculate acquisition scores based on the estimated log-likelihood ratio + log-prior. This modification made sense at the time since the option was not needed in other acquisition functions. However additive or multiplicative costs could also be used to take into account other information like variable simulation costs or unknown constraints. For example, when we consider robustness to failed simulations, one approach is to scale acquisition values with predicted success probabilities. For this reason, it would be nice to have the option available in all acquisition functions.

We could update all acquisition functions to accept and use additive_cost and multiplicative_cost, but I was worried that this would only clutter the acquisition functions when it's not clear know how much use this option will have. Hence this idea that would allow either additive or multiplicative cost (*) to be combined with any acquisition function without modifications in the acquisition function. Let me know what you think!

(*) renamed to adjustment since our current and planned examples (log-prior and success probabilities) are not really costs

Please make sure

• You have read contribution guidelines
• You have updated CHANGELOG.rst
• You have listed the copyright holder for the work you are submitting (see next section)

If your contribution adds, removes or somehow changes the functional behavior of the package, please check that

• You have included or updated all the relevant documentation, including docstrings
• You have added appropriate functional and unit tests to ensure the new features behave as expected
• You have run make lint, make docs and make test

and the proposed changes pass all unit tests (check step 6 of CONTRIBUTING.rst for details)

Copyright and Licensing

Please list the copyright holder for the work you are submitting (this will be you or your assignee, such as a university or company):

By submitting this pull request, the copyright holder is agreeing to license the submitted work under the following licenses:

• Code: BSD3 (https://opensource.org/licenses/BSD-3-Clause)
• Documentation: CC-BY 4.0 (https://creativecommons.org/licenses/by/4.0/)

[hpesonen]

Why put AdditiveAcquisition inside make_additive_acq? This seems backwards. Could it be e.g. AdjustedAcquisition class or sth for additive and multiplicative adjustments?

[uremes]

AdditiveAcquisition is inside make_additive_acq because the idea is that make_additive_acq makes a new acquisition class that inherits the base acquisition class provided as input.

if your comment was about the new acquisition class name, i can rename both AdditiveAcquisition and MultiplicativeAcquisition to AdjustedAcquisition. the current names are indeed not clear and can be misunderstood.

if the comment was about the approach in general, the reason i used dynamic inheritance is that our available acquisition classes have varied implementations for the acquire method and the adjusted acquisition class needs to use the same acquire method as the base class. but maybe there is a better way to do this?

[hpesonen]

Fair enough, it's an implementation that works for this case. We just have to be careful where and when it's being called. I guess it generates a new class every time it's being called? Even with the same AcquisitionBase-instances?

[uremes]

yes, new class every time. i understand that creates some overhead compared to predetermined classes, but i assumed that's fine since we don't call this method more than once per inference task.

[hpesonen]

Yes! Just have to make sure that users won't try out silly things with it :)

[hpesonen]

Why have AdjustmentFunctionclass and then define the maker functions later on that return classes defined with the scope of the maker functions?

[uremes]

the maker functions were needed to make new acquisition classes that inherit from the base acquisition class that is given as input. the functions currently take as input the base acquisition class and an AdjustmentFunction instance, but i could also remove this convenience class and have make_additive_acq and make_multiplicative_acq take as input the callable that we want to use as additive or multiplicative term and the callable that returns its gradient. is that what you meant?

[hpesonen]

Fair enough, it's an implementation that works for this case. We just have to be careful where and when it's being called. I guess it generates a new class every time it's being called? Even with the same AcquisitionBase-instances?