Fix serious bug in bin
#2680
Fix serious bug in bin
#2680
Conversation
| @@ -62,7 +62,7 @@ auto map_to_bins_chunkwise = [](auto &binned, auto &bins, const auto &data, | |||
| const auto i_bin = bin_indices[i]; | |||
| if (i_bin < 0) | |||
| continue; | |||
| const uint8_t j = i_bin % chunksize; | |||
There was a problem hiding this comment.
Just for my understanding, why were we only seeing the bug for number of bins larger than > 65536 if the range of uint8_t is much smaller?
Also, why is InnerIndex uint16_t instead of int16_t?
There was a problem hiding this comment.
chunksize was set as 128 for less than 128**2 bins, 256 for less than 256**2` bins, etc.
There was a problem hiding this comment.
You mean int16_t instead of uint16_t? Since int16_t is large enough for our needs there is no reason for risky mixing of signed and unsigned integer arithmetic.
Worth considering... and I think in doing so I found yet another problem with our release process: Making a release writes to the top-level docs (in addition to the one in the release's folder). That is we cannot really make a release except for the latest version (at least not without fixing things up later again). This is probably a good reason to go ahead with #2606, and change the release to either (a) not deploy any docs at all, or (b) deploy only to the release folder. |
Fixes #2679.