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.

Sign up for GitHub

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

Jump to bottom

compiler: Remove counters from subdomains and automatically generate unique names #2431

Merged
merged 17 commits into from
Oct 3, 2024
Merged

compiler: Remove counters from subdomains and automatically generate unique names #2431

merged 17 commits into from
Oct 3, 2024

Conversation

EdCaunt
Copy link
Contributor

@EdCaunt EdCaunt commented Jul 30, 2024

Rather than using counters to ensure subdimensions etc are unique, they should be dynamically renamed during compilation.

Supersedes #2405 .

@codecov Codecov
Copy link

codecov bot commented Jul 30, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 98.18182% with 3 lines in your changes missing coverage. Please review.

Project coverage is 87.07%. Comparing base (28e5ef2) to head (6ed3979).
Report is 18 commits behind head on master.

Files with missing lines Patch % Lines
devito/ir/equations/algorithms.py 95.52% 0 Missing and 3 partials ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #2431       /-   ##
==========================================
  Coverage   87.00%   87.07%    0.06%     
==========================================
  Files         239      239              
  Lines       45028    45117       89     
  Branches     8406     8427       21     
==========================================
  Hits        39177    39285      108     
  Misses       5116     5100      -16     
  Partials      735      732       -3

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

georgebisbas
georgebisbas reviewed Jul 30, 2024
View reviewed changes
devito/ir/equations/algorithms.py Show resolved Hide resolved
devito/ir/equations/algorithms.py Show resolved Hide resolved
devito/types/grid.py Show resolved Hide resolved
@review-notebook-app Review Notebook App
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

FabioLuporini
FabioLuporini reviewed Jul 31, 2024
View reviewed changes
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.

This is looking so good

devito/ir/equations/algorithms.py Outdated Show resolved Hide resolved
devito/ir/equations/algorithms.py Show resolved Hide resolved
devito/ir/equations/algorithms.py Outdated Show resolved Hide resolved
devito/ir/equations/algorithms.py Outdated Show resolved Hide resolved
devito/ir/equations/algorithms.py Outdated Show resolved Hide resolved
devito/ir/iet/utils.py Outdated Show resolved Hide resolved
devito/passes/iet/definitions.py Outdated Show resolved Hide resolved
devito/types/basic.py Outdated Show resolved Hide resolved
devito/types/basic.py Outdated Show resolved Hide resolved
devito/types/basic.py Outdated Show resolved Hide resolved
ZoeLeibowitz
ZoeLeibowitz reviewed Jul 31, 2024
View reviewed changes
devito/ir/equations/algorithms.py Outdated Show resolved Hide resolved
devito/ir/equations/algorithms.py Outdated Show resolved Hide resolved
tests/test_dimension.py Outdated Show resolved Hide resolved
tests/test_subdomains.py Outdated Show resolved Hide resolved
mloubout
mloubout reviewed Jul 31, 2024
View reviewed changes
devito/ir/equations/algorithms.py Outdated Show resolved Hide resolved
devito/types/basic.py Outdated Show resolved Hide resolved
mloubout
mloubout reviewed Jul 31, 2024
View reviewed changes
devito/types/basic.py Outdated Show resolved Hide resolved
This was referenced Aug 1, 2024
Merged
Closed
mloubout
mloubout reviewed Aug 6, 2024
View reviewed changes
devito/types/basic.py Outdated Show resolved Hide resolved
@EdCaunt EdCaunt force-pushed the counters_redux branch 2 times, most recently from 4f18c77 to de56705 Compare September 16, 2024 10:53
FabioLuporini
FabioLuporini reviewed Sep 26, 2024
View reviewed changes
devito/ir/equations/algorithms.py Outdated Show resolved Hide resolved
devito/ir/support/symregistry.py Show resolved Hide resolved
devito/ir/equations/algorithms.py Outdated Show resolved Hide resolved
devito/ir/equations/algorithms.py Outdated Show resolved Hide resolved
devito/ir/equations/algorithms.py Outdated Show resolved Hide resolved
@FabioLuporini
Copy link
Contributor

GTG!

@EdCaunt
compiler: Generate subdimension ids during compilation
e22260f
@EdCaunt
compiler: Rework concretize_subdomains
4c124e1
@EdCaunt
dsl: Rebuild SubDomainSet functions
14fbd23
@EdCaunt
compiler: Add handling for ConditionalDimension
ef0e32c
@EdCaunt
compiler: Simplify concretize_subdims
e59bcc5
@EdCaunt
compiler: Tweak subdim concretisation
a704295
@EdCaunt
compiler: Remove unneeded machinery from concretize_subdims
cb9fbd8
@EdCaunt
compiler: Introduced additional mappings for indexed Arrays
54d07b2
@EdCaunt
tests: Add tests for subdimension renaming
e8c281e
@EdCaunt
examples: Update subdomain notebook
053c70e
@EdCaunt
compiler: Avoid reassigning data during MultiSubDimension rebuilding
8b1950b
@EdCaunt
compiler: Ensure MultiSubDimension functions have unique names
8026a03
@EdCaunt
compiler: Refactor MultiSubDimension concretisation
45f87e8
@EdCaunt
compiler: Remove unnecessary multisubdimension rebuild args
4730af0
@EdCaunt
misc: Fix use of ExternalAllocator
7f43aa9
@EdCaunt
compiler: Fix issue where untouched data would be petrified upon rebu…
d7de9e0 
…ilding
@EdCaunt
compiler: Tweak renaming to operate after first encountered and warn …
6ed3979 
…user
@mloubout mloubout merged commit db692d2 into master Oct 3, 2024
31 checks passed
@mloubout mloubout deleted the counters_redux branch October 3, 2024 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mloubout mloubout mloubout approved these changes

@ZoeLeibowitz ZoeLeibowitz ZoeLeibowitz left review comments

@FabioLuporini FabioLuporini FabioLuporini approved these changes

@georgebisbas georgebisbas Awaiting requested review from georgebisbas

Assignees
No one assigned
Labels
compiler
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@EdCaunt @FabioLuporini @mloubout @georgebisbas @ZoeLeibowitz