Inconsistent use of is_mc property

[AdrianSosic]

The semantics of is_mc need to be consistently (re-)defined. Originally, it was used to indicate if an acquisition function is capable of batch optimization, but with the newer code features this definition is no longer accurate (see here)

[Scienfitz]

Hi @AdrianSosic
I looked at usages of this property and these are all usage types:

1. check whether the corresponding botorch acqf supports batching
2. check whether the corresponding botorch acqf supports pending_X
3. check whether the baybe acqf is an MC one (from the inner functionality)
4. check whether the corresponding botorch acqf is derived from the MC base class

Re 3 and 4: Those are imo not the same. From the baybe perspective eg qNIPV is a MC acqf just like the others, e.g. it supports pending_X etc. But is is not derived from the botorch MC base class but from the more general acqf base class

Re 3: This is only done in the tests for now but I think whats really intended by those is to check for 1. So if they are slightly reworked 3 is not a required property we need to enable.

Re 4: This is the attempted usage in #465 but other than that the property has never been used for that. Depending on the implementation choice below it could either become a property or we just require that this check always has to be done via isinstance

Implementation choice for a Fix:
There are two variants. In both variants I would probably make three new properties

• supports_batching
• supports_pending_experiments
• is_botorch_mc or similar

Variant A) properties dont load the botorch objects and still sue the currently implemented name check. Afaik it will remain the same for 1 and 2 for all classes except qTS which would still override the properties. 4 would then have to be done via isinstance and not become a property OR be done via the same name check that is overwritten for qNIPV.
Variant B) properties load the corresponding botorch objects and perform the checks. Eg by inspecting the constructor for 2 and so on. But I'm unsure whether that would cause some unintended lazy loading issues.

[AdrianSosic]

Hey @Scienfitz, thanks for this very detailed investigation. I think you have a very good overview at the moment, so I'd generally follow your preferences. However, I think Variant B indeed suffers from lazy-loading issues --> you'd not be able to implement this as a property. It would need to become a method, which somehow appears odd to me. So I guess a non-loading variant is the better option. It is not entirely clear to me, though, how the properties would actually be implemented. Will we need to hardcode?

I think the fastest way to converge would be if you created a corresponding draft so that we can actually see the necessary changes.

[Scienfitz]

Ok I can draft something.

From what I understand at this moment it would mean

1. remains based on the name check. qTS overwrites the property.
2. remains based on the name check.
3. likely not needed as tests will be rephrased
4. gets no property and isinstance checks are needed locally (as implemented right now)