Inconsistent constraint signs

[yunshengtian]

Hi,

In ConstrainedBaseTestProblem, constraints are expressed in the form of c(x) >= 0, while in ConstrainedMaxPosteriorSampling, constraints have the form of c(x) <= 0.

Is it meant to be the case? If so, could we use these two classes together?

Thanks!

[Balandat]

Thanks for the observation and raising this point. I believe when it comes to modeling constraints (rather than defining test problems), our convention is relatively consistent that negative value of black box constraint function imply feasibility (see also https://github.com/pytorch/botorch/blob/main/botorch/acquisition/objective.py#L454-L457). I'm not sure why this isn't the case for the test problems, but I guess it's at least partially due to the conventions in the references from which we adopted the test problems.

That said, you can certainly use the test problems together with ConstrainedMaxPosteriorSampling, just be mindful that when modeling the constraint outcomes to flip the sign of the observation.

We should probably consider unifying the conventions here (though it might not be straightforward to do this in a backward-compatible fashion). cc @saitcakmak, @esantorella

[yunshengtian]

Thanks for the quick reply! Unifying to negative = feasible sounds like a good idea.