New acquisition functions

[Scienfitz]

Some progress towards refactoring the acqf interface and enabling more advanced acqfs

Done

• removed debotorchize
• incldued some new acqfs, see CHANGELOG
• enable iteration tests with all acqfs
• extended hypothesis tests

Not Done Yet:

• removing AdapterModel (does not make sense while Refactor surrogates and decorators #209 is open)

Issues:

• some tests with custom surrogates fail, see separate thread resolved since not using botorch factory anymore
• some of the analytical new acqfs dont seem to work wiht out GP model. Eg when I implement the NEI I get botorch.exceptions.errors.UnsupportedError: Only SingleTaskGP models with known observation noise are currently supported for fantasy-based NEI & LogNE. Also the LogPI is available in botorch, but it is not imported to the top level acqf package in botorch, I ignored it here. I included a reverted commit pair so it can be checked out quickly to reproduce

[AdrianSosic]

Hi @Scienfitz, thanks for this tricky PR. Here already the first chunk of comments so that you have something to work on while I review the rest

[AdrianSosic]

I think it actually makes sense to remove the try-catch and instead raise this error in a guard clause at the top under the appropriate condition (i.e. batch_size>1 and non-MC function). Because I've already had weird situations several times that a different AttributeError was thrown (while developing) and this got me completely off track. And I don't see a reason really why should wait for the optimization to fail... Right?

[Scienfitz]

yes this atttribute error ctach ehre can have several rasons and ive also encountered it for some of the (not included) new acqf functions that complain about something else than our error here. It def needs to be fixed

[Scienfitz]

having said that the batch size change you suggest would not be enough for that

[AdrianSosic]

What you you mean with batch size change? My suggestion was simply to have an if-condition that checks for batch_size>1 AND not is_mc ...

[Scienfitz]

yes i could do that but it wont solve any of the other issues that could cause different attribute errors

[AVHopp]

Yes, and we can just explicitly link to the original function and then add stuff if that is fine with our linters

[Scienfitz]

I propose the following

        """Generate recommendations from a discrete search space.

        Note: This method overwrites
        :func:`baybe.recommender.pure.base.PureRecommender._recommend_discrete`
         keeping the same characteristics except the ones mentioned here.

        Raises:
            NoMCAcquisitionFunctionError: If a non-Monte Carlo acquisition function is
                used with a batch size > 1.
        """

its part of this PR via amending the last commit
keep in mind this is wont shown in the doc anyway because its a private function
please let me know if that can go ahead

I notice also inconsistent behavior here, in the same file we have _recommend_hybrid which also overwrites the base class implementation, but has its own completely copypasta docstring with an amended raises section

[Scienfitz]

never mind its not possible to do that without violating lots of stuff

I would simply follow the approach of _recommend_hybrid

[Scienfitz]

and the same problems were also part of conti and hyprid error raising so I changed it there too
Seems our previous workflow was to copy the entire docstirng+amend which I did for those as well

[AdrianSosic]

Ok, I'm not a happy guy but I think there is simply no "good" approach, so I'll simply look away 🙈

[AdrianSosic]

Overall, looking great 👍🏼 Here the remaining comments