Skip to content

Conversation

@kieronqtran
Copy link

@kieronqtran kieronqtran commented Oct 17, 2025

This pull request makes a small but important fix to the random value assignment logic in the gecode/flatzinc/flatzinc.cpp file. The change ensures that the generated random integer values can include the upper bound of the specified range, correcting an off-by-one error. This is happened becasue Rnd _random input domain is [lower_bound, upper_bound) and + 1 to correct set to [lower_bound, upper_bound], and it is effect to uniform_on_restart constraints.

  • Fixed the calculation of random values for uniform integer ranges to include the upper bound by changing the random range from (range.second - range.first) to (range.second - range.first + 1).

@Dekker1
Copy link
Contributor

Dekker1 commented Oct 26, 2025

This fix looks good to me. As you pointed out, I didn't see that the Rnd::operator() gives a value in the range [0,n) instead of [0,n] as I expected.

The same issue actually is also present for floating point, although in a different form:

const FloatVal rndVal = (static_cast<FloatVal>(_random(INT_MAX)) / INT_MAX)*(range.second - range.first) + range.first;

This will never reach the upper bound, as the division will never reach one because INT_MAX is excluded. To fix this, we might have to give up some of the granularity in the division and reduce the right-hand side of the division:

const FloatVal rndVal = (static_cast<FloatVal>(_random(INT_MAX)) / (INT_MAX-1))*(range.second - range.first) + range.first;

This seems fine, and I think any other fix would require an inclusive version of the random number generator, which would be more work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants