-
Notifications
You must be signed in to change notification settings - Fork 6.8k
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@mxnet-label-bot Add [Test] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thank you!
Thanks, needs mshadow PR to be merged first. Then I remove [WIP], rerun CI and merge. |
@mxnet-label-bot add [pr-awaiting-review] |
@mxnet-label-bot add[pr-awaiting-merge] |
@mxnet-label-bot remove[pr-awaiting-response, pr-awaiting-review] |
@mxnet-label-bot remove[pr-work-in-progress] |
@larroy Please rebase so it can be merged |
I don't think this change passes CI. I can either have another look in my next oncall or close. I don't have time in the following days to continue working on this issue. |
@mxnet-label-bot remove [pr-awaiting-merge] |
@larroy What's the path forward here ? |
I don't think I have time to look into this, so I might have to close. |
Description
Add a test for big matrix multiplications that overflow gemm int32 indexes.
This test checks that an exception is thrown instead of calling gemm with wrong indices which causes segfaults or wrong results.
For more info see related issues and PR:
#14522 dmlc/mshadow#372
Checklist
Essentials
Please feel free to remove inapplicable items for your PR.