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

dsl: Patch domain decomposition bug with SubDomains #2246

Merged
merged 1 commit into from
Oct 28, 2023
Merged

Conversation

EdCaunt
Copy link
Contributor

@EdCaunt EdCaunt commented Oct 25, 2023

No description provided.

@EdCaunt EdCaunt requested a review from FabioLuporini October 25, 2023 11:30
@codecov
Copy link

codecov bot commented Oct 25, 2023

Codecov Report

Merging #2246 (5f026de) into master (27c3ac2) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #2246   +/-   ##
=======================================
  Coverage   86.90%   86.91%           
=======================================
  Files         229      229           
  Lines       41759    41790   +31     
  Branches     7703     7709    +6     
=======================================
+ Hits        36292    36323   +31     
  Misses       4840     4840           
  Partials      627      627           
Files Coverage Δ
devito/data/decomposition.py 85.78% <100.00%> (+0.27%) ⬆️
devito/mpi/distributed.py 92.90% <100.00%> (+0.02%) ⬆️
devito/types/grid.py 92.66% <100.00%> (+0.02%) ⬆️
tests/test_subdomains.py 100.00% <100.00%> (ø)

Copy link
Contributor

@FabioLuporini FabioLuporini left a comment

Choose a reason for hiding this comment

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

good stuff

@@ -162,6 +162,66 @@ def define(self, dimensions):

assert u0.data.all() == u1.data.all() == u2.data.all() == u3.data.all()

sd_specs = [None, ('middle', 1, 7), ('middle', 2, 3), ('middle', 7, 1),
Copy link
Contributor

Choose a reason for hiding this comment

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

Need a ('middle', 5, 5) case for the case it's fully contained in the center domain and make sure you have 3 domains along that dimension

Copy link
Contributor

@mloubout mloubout left a comment

Choose a reason for hiding this comment

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

Squash the debug commits into one and GTG

types: subdimension bounds, decomposition handling

tests: Added test for SubDomains and domain decomposition

tests: Removed unnecessary MPI mode specification

tests: Added diagnostic print statements

tests: Fixed diagnostic print statements

tests: fixed occasional None in substitution

tests: Fixed typo

tests: Made Subdomain + MPI data check global rather than per-rank

tests: Removed unecessary complexity from MPI + SubDomains test
@mloubout mloubout merged commit 5e4e14c into master Oct 28, 2023
32 checks passed
@mloubout mloubout deleted the subdim-mpi-fix branch October 28, 2023 11:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants