Repository navigation
[SYSTEMDS-3891] Add OOC Reshape Primitive - #2619
jessicapriebe wants to merge 9 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2619 +/- ##
============================================
+ Coverage 71.41% 72.13% +0.71%
- Complexity 50721 51130 +409
============================================
Files 1642 2097 +455
Lines 197165 226729 +29564
Branches 38306 38627 +321
============================================
+ Hits 140815 163556 +22741
- Misses 45279 51992 +6713
- Partials 11071 11181 +110
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Baunsgaard
left a comment
There was a problem hiding this comment.
Just because i was curious on the OOC operator i took a look. However, it was not super decipherable from the PR for me, so I thought I would leave some hints to clean it up and make it readable.
All just suggestions.
|
Thanks for the patch @jessicapriebe! I reviewed the PR and found some issues related to asynchronous execution and error propagation. I opened a PR to (hopefully) address those issues. Since your PR is fairly complex, I leave those proposed changes for you to review :) Generally, there is some duplicate code for row/col specific handling. Not sure if this can somehow be simplified but just as a side note. Also, indexing is still hard to understand but I'm honestly not sure how to fix this. Maybe by wrapping those computations into more descriptive functions with a block comment? |
Fix Async Processing Issues and Improve Exception Handling
|
Thanks for the feedback @janniklinde! I included your proposed changes with some minor adjustments. I also cleaned up the indexing and added comments for the more complex parts, while keeping the row/column-specific handling mostly separate for readability. |
janniklinde
left a comment
There was a problem hiding this comment.
Thanks for the update @jessicapriebe. I left three more comments where I still have correctness concerns (which I missed before)
This adds an OOC reshape primitive that reshapes matrices out-of-core by splitting input blocks into slices and storing them in the cache-owned state table. Output blocks are rebuilt individually or as block rows/columns, depending on the reshape dimensions, and are enqueued as soon as they are completed.