Skip to content
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

mpi: Fix haloupdate with inner dim #2264

Closed
wants to merge 2 commits into from
Closed

mpi: Fix haloupdate with inner dim #2264

wants to merge 2 commits into from

Conversation

mloubout
Copy link
Contributor

@mloubout mloubout commented Nov 10, 2023

Fixes #2262

@mloubout mloubout added MPI mpi-related compiler labels Nov 10, 2023
Copy link

codecov bot commented Nov 10, 2023

Codecov Report

Attention: 67 lines in your changes are missing coverage. Please review.

Comparison is base (d0cd354) 86.92% compared to head (f37445e) 77.76%.
Report is 2 commits behind head on master.

Files Patch % Lines
tests/test_mpi.py 0.00% 54 Missing ⚠️
devito/ir/stree/algorithms.py 64.00% 7 Missing and 2 partials ⚠️
devito/ir/iet/algorithms.py 50.00% 2 Missing ⚠️
devito/passes/iet/mpi.py 0.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2264      +/-   ##
==========================================
- Coverage   86.92%   77.76%   -9.17%     
==========================================
  Files         229      229              
  Lines       42150    42230      +80     
  Branches     7787     7802      +15     
==========================================
- Hits        36639    32839    -3800     
- Misses       4863     8694    +3831     
- Partials      648      697      +49     

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

else:
hispace = None

if hispace and options['mpi']:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@FabioLuporini replying to your comment in the other PR

We only check options['mpi'] here to avoid creating empty loops without mpi since the haloupdate is then dropped, the halo is still attached the normal way in the else when options['mpi'] = False so it's still there.

@FabioLuporini FabioLuporini changed the title MPI: Fix haloupdate with inner dim mpi: Fix haloupdate with inner dim Nov 16, 2023
@@ -601,6 +602,94 @@ def test_precomputed_sparse(self, r):
Operator(sf1.interpolate(u))()
assert np.all(sf1.data == 4)

@pytest.mark.parallel(mode=1)
Copy link
Contributor

Choose a reason for hiding this comment

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

memo for me: probably mode=4

op.apply()
assert np.all(s.data == 1)

@pytest.mark.parallel(mode=1)
Copy link
Contributor

Choose a reason for hiding this comment

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

same, mode=4 ?

@pytest.mark.parallel(mode=1)
def test_sparse_first(self):
"""
Tests custom sprase function with sparse dimension as first index.
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo "sprase"

"""

class SparseFirst(SparseFunction):
""" Custom sparse class with the sparse dimension as the first one"""
Copy link
Contributor

Choose a reason for hiding this comment

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

Space floating around at start of docstring

@mloubout
Copy link
Contributor Author

superseded by #2272 , closing

@mloubout mloubout closed this Nov 21, 2023
@FabioLuporini FabioLuporini deleted the halo-inner-dim branch November 22, 2023 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler MPI mpi-related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

mpi not supported for class of sparse functions
4 participants