Correct C++ floor division for integers #1496
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes #1495. As mentioned in the issue, the problem is that we use
floor(1.0*x/y)
to calculate floor division with Python semantics in C++. By default, we have unsafe math optimizations enabled, which leads to this not always being correct (e.g. 98/98 would be rounded down to 0). I think this is due to replacing the division by multiplication with an approximated reciprocal, or something along these lines. Switching off unsafe optimizations would be an option, but it would slow down code in other places.In this PR, I instead create function specializations for integer arguments, which then use a rather complex calculation to get the value:
This is the implementation used by Cython (
cdivision: false
, obviously), and it should cover all corner cases. I didn't benchmark this, but it is probably quite a bit slower than the previous solution. This could bring up #993 on the table (the issue is about Cython, but the same is now relevant for C++), but on the other hand all use cases I can think of for floor division and modulo operations are initializations andrun_regularly
operations, nothing that is time-critical. As a data point, all usages of//
in our examples are for initializations likex = i // columns
.