-
Notifications
You must be signed in to change notification settings - Fork 54
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
Implement distributed unfold
operation
#1419
Implement distributed unfold
operation
#1419
Conversation
Thank you for the PR! |
…ilar_to_torch_Tensor_unfold
Thank you for the PR! |
1 similar comment
Thank you for the PR! |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1419 +/- ##
==========================================
+ Coverage 92.04% 92.07% +0.02%
==========================================
Files 83 83
Lines 12113 12144 +31
==========================================
+ Hits 11150 11181 +31
Misses 963 963
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
The tests on the CUDA-runner seem to hang at |
…> chunk_size more tests
…ilar_to_torch_Tensor_unfold
Thank you for the PR! |
Thank you for the PR! |
Thank you for the PR! |
Thank you for the PR! |
1 similar comment
Thank you for the PR! |
Thank you for the PR! |
On the Terrabyte cluster, using 8 processes on 2 nodes with 4 GPUs each I get the following error:
on CPU, everything seems to work (at least in |
…ilar_to_torch_Tensor_unfold
@mrfh92 I have now added the error for the case that size=1. I could also verify that the synchronization errors that caused data corruption do not occur anymore, so this PR should be ready for merging. |
Undid my stupid change before that belongs to another issue
Thank you for the PR! |
…ilar_to_torch_Tensor_unfold
Thank you for the PR! |
…ilar_to_torch_Tensor_unfold
Thank you for the PR! |
@FOsterfeld from my point of view this now looks fine |
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.
Looks fine from my point of view. @FOsterfeld Thanks 👍
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.
@FOsterfeld @mrfh92 this looks great, I only found some (presumably) dead code that can be removed, otherwise I think it can be merged. Thanks a lot!
…ilar_to_torch_Tensor_unfold
Thank you for the PR! |
Thank you for the PR! |
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.
Great job @FOsterfeld !
…ilar_to_torch_Tensor_unfold
Thank you for the PR! |
unfold
operation
…ilar_to_torch_Tensor_unfold
Thank you for the PR! |
Due Diligence
Description
Add the function unfold to the available manipulations. unfold(a, dimension, size, step) for a DNDarray a behaves like torch.Tensor.unfold.
Example:
Issue/s resolved: #1400
Changes proposed:
Type of change
Memory requirements
Performance
Does this change modify the behaviour of other functions? If so, which?
no