-
Notifications
You must be signed in to change notification settings - Fork 59
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[torchlib] Fix index_put: handle None cases #2061
Conversation
858d31a
to
9e895f7
Compare
@microsoft-github-policy-service agree company="Brainchip" |
@justinchuby PR is ready for review |
9e895f7
to
94d820a
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2061 +/- ##
==========================================
- Coverage 72.35% 72.24% -0.11%
==========================================
Files 216 217 +1
Lines 28890 29053 +163
Branches 3433 3449 +16
==========================================
+ Hits 20904 20990 +86
- Misses 6856 6934 +78
+ Partials 1130 1129 -1 ☔ View full report in Codecov by Sentry. |
Thank you! Could you look at the failing tests? |
94d820a
to
17e963b
Compare
Force pushed as I changed a little bit the logic. Also added more test cases. |
Could you take a look at the falling CI, and we can merge this PR? Thanks! |
fe45589
to
3d7ef85
Compare
it was missing a case with multidimensional index (added a test for that case). Tell me if other cases are missing that you want to support. |
3d7ef85
to
f235c4a
Compare
For range, I think we need to Squeeze the tensor to remove its dimension. |
@justinchuby I wasn’t able to make this work on symbolic tensors. Please feel free to close the draft or continue from here if helpful. |
Thanks. Could you share the issues you have seen? I can take it from here |
Will merge as-is and create follow up improvements |
This PR introduces support for
None
indices in theindex_put
function. If an index is None, it acts as a full slice (:
).index_put Logic: