Batch of fixes for index overflows in grid stride loops. #10448
Conversation
…strings() kernels.
nvdbaranec
added
bug
Codecov Report
@@ Coverage Diff @@
## branch-22.04 #10448 +/- ##
================================================
+ Coverage 86.13% 86.17% +0.03%
================================================
Files 139 141 +2
Lines 22438 22501 +63
================================================
+ Hits 19328 19391 +63
Misses 3110 3110
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
I'm approving because this fixes bugs and would improve the stability/functionality of the 22.04 release. There's still some further investigation needed to make these kinds of changes more thoroughly through the code base and/or adopt alternative strategies such as range-based for loops or a grid helper, but those shouldn't hold back the 22.04 release and can be done in later PRs.
|
Just as a note: I'm currently fighting the jit compiler trying to change a couple of the casts as discussed with @bdice. It's either crashing or failing to compile with no output for some reason. |
|
rerun tests |
|
@gpucibot merge |
1 similar comment
|
@gpucibot merge |
Partially addresses #10368
Specifically:
valid_ifscatterrolling_windowcompute_column_kernel(ast stuff)replace_nulls(fixed-width and strings)The majority of the fixes are simply making the indexing variable a
std::size_tinstead of acudf::size_type. Although scatter had an additional place it was overflowing outside the kernel.I didn't add tests for these fixes, but each of them were individually tested locally to make sure they actually manifested the issue and then were verified with the fixes.