[BUG] Bounds are not checked on variable definition

[arnaud-m]

Describe the bug
The lower and upper bounds are not checked on variable definition.

To Reproduce
Define a variable with invalid bounds.

m.intVar( 2 * IntVar.MIN_INT_BOUND, 2 * IntVar.MAX_INT_BOUND); 

Expected behavior

I am unsure, but the behavior should depend on the nature of the variable : decision or auxiliary.

• One can ask the user to bound decision variables.
• However, auxiliary variables can be assigned large values even in a small search space. It is the case for a product or power constraints, or more generally arithmetic expressions.

For now, the practical issue is that the behavior is hard to predict.
Some later checks can raise an exception, or not, when building a model with arithmetic expressions.

[cprudhom]

I believe a first step is to make the bound checker correct.
Indeed, when creating an integer variable, the following code is always executed:

default void checkIntDomainRange(String NAME, int MIN, int MAX) {
        if (MIN <= Integer.MIN_VALUE || MAX >= Integer.MAX_VALUE) {
            throw new SolverException(NAME + ": consider reducing the bounds to avoid unexpected results");
        }
        if (MAX < MIN) {
            throw new SolverException(NAME + ": wrong domain definition, lower bound > upper bound");
        }
    }

but, the first condition is only detected when the lower (resp. upper) bound is equal to Integer.MIN_VALUE (resp. Integer.MAX_VALUE). The comparison should be done against IntVar.MIN_INT_BOUND (resp. IntVar.MAX_INT_BOUND).

Now, talking about auxiliary or decision variables is another topic, I believe.

[arnaud-m]

Now, talking about auxiliary or decision variables is another topic, I believe.

Yes. Could there be a setting to disable the checks because the constants are low ?

[jgFages]

I am not sure to follow but in my understanding, IntVar.MAX_INT_BOUND was only a help to have a good behavior by default, but should not be a hard restriction. I have many problems involving variables bigger than this value.

I am fine with the current implementation of checkIntDomainRange relying on Integer.MAX_VALUE and I would not like it to be changed to IntVar.MAX_INT_BOUND.

Integer.MAX_VALUE is already quite limitating, we should not decrease it even more.

[cprudhom]

IntVar.MAX_INT_BOUND was only a help to have a good behavior by default, but should not be a hard restriction.

I agree that hard restriction might be too much, but at least it informs users on possible under/overflow issues.
That said, only a subset of constraints can meet that issues, they could be equipped with checker directly, instead of checking any domain.

I am fine with the current implementation of checkIntDomainRange relying on Integer.MAX_VALUE

The current checker is rather useless: it only forbids [Integer.MIN_VALUE, Integer.MAX_VALUE], but allows [Integer.MIN_VALUE+1, Integer.MAX_VALUE-1].
Keeping that checker is conditional on not listing constraint with under/overflow issues.

Integer.MAX_VALUE is already quite limitating, we should not decrease it even more.

It is limited when considering the arithmetic constraints, mainly (only?).

[cprudhom]

Another proposal:

 default void checkIntDomainRange(String NAME, int MIN, int MAX) {
        if (MIN == Integer.MIN_VALUE || MAX == Integer.MAX_VALUE) {
            if (ref().getSettings().warnUser()) {
                ref().getSolver().log().red().printf("%s : consider reducing the bounds to avoid unexpected results", NAME);
            }
        }
        if (MAX < MIN) {
            throw new SolverException(NAME + ": wrong domain definition, lower bound > upper bound");
        }
    }

[jgFages]

this looks good to me, but if it is a log and not an exception, maybe we can even use if max > IntVar.MaxIntBound.