change the type of u to prevent illegal memory access #722
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.
I was applying implicit's AlternatingLeastSquares with CUDA on a matrix of shape (4091537, 5256984). The number of factors was set to 1011.
In doing so, AlternatingLeastSquares was failing with the following traceback:
The last line in the stack trace is a follow-up error referring to the destructor of CSRMatrix.
I tracked the original error down to the CUDA kernel. To be precise, to the lines 38 and 39 in als.cu (referring to implicit version 0.7.2):
Both
u
andfactors
are of typeint
andu
may go up touser_count-1
. Considering the aforementioned matrix dimensions, there are values foru
where this product exceeds the maximum value for integers (2147483647) in C++ leading to a negative array index.By using
size_t
as type foru
, this can be prevented.Side note: it is remarkable that the second call to
least_squares
(line 162 in als.py) is failing even though the first call (line 159 in als.py) already produces negative array indexes.