facebookresearch / nevergrad

A Python toolbox for performing gradient-free optimization
https://facebookresearch.github.io/nevergrad/
MIT License
3.95k stars 353 forks source link

Constraints don't work #1505

Closed mstniy closed 1 year ago

mstniy commented 1 year ago

Steps to reproduce

import nevergrad as ng
parameterization=ng.p.Instrumentation(ng.p.Scalar(lower=-100, upper=100))
opt= ng.optimizers.NGOpt(parameterization, budget=100)
opt.minimize(lambda arg: (arg-5.67)**2, constraint_violation=[lambda args:7-args[0][0]])

Observed Results

Nevergrad suggests 5.676

Expected Results

7

Relevant Code

Python version: 3.10.6 Nevergrad version: 0.6.0

nhansendev commented 1 year ago

Normally constraints are of the format x >= 1, for example, which cleanly evaluates to true or false. Could you clarify what your constraint is intended to do, please?

The constraint as written will always be true unless args[0][0] == 7:

for i in range(10):
    print(bool(7 - i))

True
True
True
True
True
True
True
False
True
True
mstniy commented 1 year ago

Nevergrad allows the use of float constraints to provide richer information to the optimizer, see https://facebookresearch.github.io/nevergrad/optimization.html#optimization-with-constraints

Note that the documentation seems to be outdated/wrong in mentioning that >=0 means OK, see https://github.com/facebookresearch/nevergrad/blob/8087bca851f5538f17f8a06a418ca77a5604c700/nevergrad/optimization/base.py#L630

nhansendev commented 1 year ago

Nevergrad allows the use of float constraints to provide richer information to the optimizer, see https://facebookresearch.github.io/nevergrad/optimization.html#optimization-with-constraints

Note that the documentation seems to be outdated/wrong in mentioning that >=0 means OK, see

https://github.com/facebookresearch/nevergrad/blob/8087bca851f5538f17f8a06a418ca77a5604c700/nevergrad/optimization/base.py#L630

Ah, my bad. I totally missed the float part.

if constraint_violation is not None:
            if penalty_style is not None:
                a, b, c, d, e, f = penalty_style
            else:
                a, b, c, d, e, f = (1e5, 1.0, 0.5, 1.0, 0.5, 1.0)

            violation = float(
                (a + np.sum(np.maximum(loss, 0.0)))
                * ((f + self._num_tell) ** e)
                * (b * np.sum(np.maximum(constraint_violation, 0.0) ** c) ** d)
            )
            loss += violation

Looking at the code for evaluating the consequences of a constraint violation shows that the in-code documentation is correct. constraint_violation values > 0 rapidly produce large losses.

Testing with the constraint changed to args[0][0]-7 produced the desired value of 7 for me. Does that address your issue?

mstniy commented 1 year ago

In the case of the snippet in the issue description the optimizer should work under the constraint x>=7, where the optimum of x=7.

Negating the constraint turns it into x<=7, and the optimum is 5.67

nhansendev commented 1 year ago

In the case of the snippet in the issue description the optimizer should work under the constraint x>=7, where the optimum of x=7.

Negating the constraint turns it into x<=7, and the optimum is 5.67

I think we are in agreement that the website documentation is wrong and actually inverted from the code implementation/documentation, right? I was just trying to clarify whether you can actually reach your desired result by changing the constraint equation, or if something else is broken. This way we can clarify if only a documentation update is needed.

mstniy commented 1 year ago

I agree that the online documentation is wrong. The optimizer itself is also broken, as it produces a result that doesn't satisfy the contraints in one case, and in the other it produces a suboptimal result.

nhansendev commented 1 year ago

After running the code again I see your point. 7-args[0][0] should work per the current code, but produces the wrong value. I was thinking about it backwards.

nhansendev commented 1 year ago

https://github.com/facebookresearch/nevergrad/blob/8087bca851f5538f17f8a06a418ca77a5604c700/nevergrad/parametrization/utils.py#L269

It looks to me like the source of the problem is here:


def float_penalty(x: tp.Union[bool, float]) -> float:
    """Unifies penalties as float (bool=False becomes 1).
    The value is positive for unsatisfied penality else 0.
    """
    if isinstance(x, (bool, np.bool_)):
        return float(not x)  # False ==> 1.
    elif isinstance(x, (float, np.float_)):
        return -min(0, x)  # Negative ==> >0
    raise TypeError(f"Only bools and floats are supported for check constaint, but got: {x} ({type(x)})")

Specifically, the -min(0, x) part, which is applied to the output of the constraint function.

print([-min(0, x) for x in [-1, 0, 1]])
>> [1, 0, 0]

These values are then checked relative to zero in the parameter.satisfies_constraints function: return all(utils.float_penalty(func(val)) <= 0 for func in self._constraint_checkers)

For the 7-args[0][0] case:

value = 3         # example
constraint_func(value) -> 4
float_penalty(4) -> 0
0 <= 0 -> True
No constraint violation!

value = 8         # example
constraint_func(value) -> -1
float_penalty(-1) -> 1
1 <= 0 -> False
Constraint violation!

So I believe either the -min(0, x) function is wrong, or the <=0 comparison is wrong.

teytaud commented 1 year ago

Thanks for pointing out this. The issue is solved by https://github.com/facebookresearch/nevergrad/pull/1554, to be merged soon and in released in 0.13.0.

teytaud commented 1 year ago

(the optimization run was ok in terms of minimize/ask/tell, but recommend was not doing the right thing)

teytaud commented 1 year ago

Presumably the issue is solved. Thanks a lot!