mpi: Support data assignment to SparseFunctions from local array with different distribution#2951
mpi: Support data assignment to SparseFunctions from local array with different distribution#2951FabioLuporini wants to merge 8 commits into
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2951 +/- ##
==========================================
- Coverage 83.47% 82.45% -1.03%
==========================================
Files 249 249
Lines 52276 52624 +348
Branches 4503 4527 +24
==========================================
- Hits 43638 43389 -249
- Misses 7880 8462 +582
- Partials 758 773 +15
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:
|
6ce82fb to
9a9b51c
Compare
76378a5 to
0b87d5d
Compare
|
|
||
| def _mpi_advanced_1d_target(self, glb_idx, axis): | ||
| """ | ||
| Return the raw local ndarray addressed by ``glb_idx`` without |
There was a problem hiding this comment.
Nitpick: This seems out of step with the function name?
There was a problem hiding this comment.
yeah, I think you're right. Fixing docstring
| """Raise the first error reported by any rank, on every rank.""" | ||
| if data._distributor.nprocs > 1: | ||
| errors = data._distributor.comm.allgather(error) | ||
| error = next((i for i in errors if i is not None), None) |
There was a problem hiding this comment.
Is it worth reporting the rank(s) that raised the error here?
| assert trace_data.flags.f_contiguous | ||
|
|
||
| rec.data_local[:, :] = 0 | ||
| rec.data[:, trace_index] = trace_data |
There was a problem hiding this comment.
Can you do the same with rec.data.coordinates?
There was a problem hiding this comment.
you cannot use advanced indexing like this here in objects that have more than 1 distributed dimension (explained in the notebook it's a current limitation)
but what would it be for?
There was a problem hiding this comment.
Well I was assuming that if you wanted to fill the data with some fancy indexing, then it's not unlikely you would want to set the coordinates of those sparse points in the same manner
No description provided.