[MRG] FIX index overflow error in sparse matrix polynomial expansion #16831
Conversation
| # use np.int64 for index datatype to prevent overflow | ||
| # in case X has a large dimension | ||
| Xp_next = _csr_polynomial_expansion(X.data, | ||
| X.indices.astype(np.int64), |
There was a problem hiding this comment.
What kind of error do you get when using the fused type without casting to np.int64?
There was a problem hiding this comment.
in my case the overflow error still occurs if no casting was used, even when the type is a fused type.
There was a problem hiding this comment.
A self-contained example code would make it easier to reproduce your issue. May you expand on the code snippet in #16803 (comment) with code to create x as well?
There was a problem hiding this comment.
@thomasjpfan I have updated the code snippet in #16803 to be self-contained and can reproduce this issue.
|
Hi @jianlingzhong, thanks for your work. Some checks didn't pass and there are conflicts with the upstream version. Do you mind having a look and fixing those issues? Thanks! |
|
Hi @jianlingzhong , are you still working on this ? (Having this PR merged would help resolve one of the tests for sparse inputs I'm working on cf. #13246 (comment)) If not, I could give it a shot, by just resolving conflicts, however I'm not familiar with cython, so I could try to reproduce the overflow error you talk about, but I don't know how hard it is to solve it... |
|
@wdevazelhes Hey sorry I was not able to spend time on this at moment. Feel free to give it a shot. |
|
Thanks @jianlingzhong , I'll give it a try ! |
|
Closing because the work is being continued at #19676 |
Reference Issues/PRs
Fixes #16803
What does this implement/fix? Explain your changes.
Change index type from
np.int32_to a fused type and explicitly cast indices tonp.int64before passed into the polynomial expansion function.Any other comments?