FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

mpi: Support data assignment to SparseFunctions from local array with different distribution by FabioLuporini · Pull Request #2951 · devitocodes/devito · GitHub

/ devito Public

mpi: Support data assignment to SparseFunctions from local array with different distribution - #2951

Closed
FabioLuporini wants to merge 8 commits into
mainfrom
sparse-data-mpi
ConversationCommits (8)ChecksFiles changed
Closed

mpi: Support data assignment to SparseFunctions from local array with different distribution#2951
FabioLuporini wants to merge 8 commits into
mainfrom
sparse-data-mpi

Conversation

FabioLuporini commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

No description provided.

FabioLuporini requested a review from mloubout June 18, 2026 15:29
FabioLuporini added the MPI mpi-related label Jun 18, 2026

review-notebook-app Bot commented Jun 18, 2026

Copy link
Copy Markdown

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

codecov Bot commented Jun 18, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 19.83003% with 283 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.45%. Comparing base (58df797) to head (9de8eb7).
⚠️ Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
devito/data/utils.py 21.52% 111 Missing and 2 partials ⚠️
tests/test_data.py 20.86% 91 Missing ⚠️
tests/test_mpi.py 10.81% 66 Missing ⚠️
devito/data/data.py 26.66% 9 Missing and 2 partials ⚠️
devito/types/dense.py 60.00% 2 Missing ⚠️
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
Flag Coverage Δ
pytest-gpu-aomp-amdgpuX 68.36% <17.68%> (-0.24%) ⬇️
pytest-gpu-gcc- 77.80% <19.83%> (-0.42%) ⬇️
pytest-gpu-icx- 77.77% <19.83%> (-0.41%) ⬇️
pytest-gpu-nvc-nvidiaX ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

FabioLuporini force-pushed the sparse-data-mpi branch 2 times, most recently from 6ce82fb to 9a9b51c Compare June 19, 2026 08:29
EdCaunt reviewed Jun 19, 2026
View reviewed changes
Comment thread devito/data/data.py Outdated

def _mpi_advanced_1d_target(self, glb_idx, axis):
"""
Return the raw local ndarray addressed by ``glb_idx`` without

EdCaunt Jun 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Nitpick: This seems out of step with the function name?

FabioLuporini Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

yeah, I think you're right. Fixing docstring

Comment thread devito/data/utils.py Outdated
"""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)

EdCaunt Jun 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Is it worth reporting the rank(s) that raised the error here?

FabioLuporini Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

ok, tweaking

Comment thread tests/test_mpi.py
assert trace_data.flags.f_contiguous

rec.data_local[:, :] = 0
rec.data[:, trace_index] = trace_data

EdCaunt Jun 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Can you do the same with rec.data.coordinates?

FabioLuporini Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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?

EdCaunt Jun 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

FabioLuporini commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

closing as superseded by #2955

FabioLuporini deleted the sparse-data-mpi branch June 30, 2026 08:40
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

EdCaunt EdCaunt approved these changes
mloubout Awaiting requested review from mloubout

Assignees

No one assigned

Labels

MPI mpi-related

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

Back | FazBrowse Home | New Git URL