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

Distribute mesh divisions using Positions #26224

Merged
merged 8 commits into from
Dec 8, 2023
Merged

Distribute mesh divisions using Positions #26224

merged 8 commits into from
Dec 8, 2023

Conversation

GiudGiud
Copy link
Contributor

@GiudGiud GiudGiud commented Dec 2, 2023

refs #26223

@GiudGiud GiudGiud self-assigned this Dec 2, 2023
@GiudGiud GiudGiud marked this pull request as ready for review December 2, 2023 02:58
@moosebuild
Copy link
Contributor

moosebuild commented Dec 2, 2023 •
edited

Job Documentation on 9c9f699 wanted to post the following:

View the site here

This comment will be updated on new commits.

@moosebuild
Copy link
Contributor

moosebuild commented Dec 2, 2023 •
edited

Job Coverage on 23b504f wanted to post the following:

Framework coverage

d450a5 #26224 23b504
Total Total +/- New
Rate 85.01% 85.01% +0.01% 93.23%
Hits 97252 97336 +84 124
Misses 17153 17157 +4 9

Diff coverage report

Full coverage report

Modules coverage

Coverage did not change

Full coverage reports

Reports

This comment will be updated on new commits.

@GiudGiud
Copy link
Contributor Author

GiudGiud commented Dec 3, 2023

Umm one concern here is overlap between bins if the positions are not far away enough from each other. I think I ll add a check in debug

lindsayad
lindsayad reviewed Dec 4, 2023
View reviewed changes
framework/src/meshdivisions/CartesianGridDivision.C Outdated Show resolved Hide resolved
framework/src/meshdivisions/CartesianGridDivision.C Outdated Show resolved Hide resolved
framework/src/meshdivisions/CartesianGridDivision.C Outdated Show resolved Hide resolved
framework/src/meshdivisions/CylindricalGridDivision.C Outdated Show resolved Hide resolved
framework/src/meshdivisions/CylindricalGridDivision.C Outdated Show resolved Hide resolved
framework/src/meshdivisions/CartesianGridDivision.C Outdated Show resolved Hide resolved
framework/src/meshdivisions/SphericalGridDivision.C Outdated Show resolved Hide resolved
test/tests/meshdivisions/tests Outdated Show resolved Hide resolved
test/tests/meshdivisions/tests Outdated Show resolved Hide resolved
test/tests/meshdivisions/tests Outdated Show resolved Hide resolved
GiudGiud and others added 3 commits December 6, 2023 15:40
@GiudGiud
Use Positions to distribute the mesh divisions, refs idaholab#26223
de0662a
@GiudGiud
Add testing for distributing mesh divisions using Positions
14b363b
@GiudGiud
Address Alex's review
4692153 
- constness
- clearer code split
- test wording
Co-authored-by: Alex Lindsay <alexander.lindsay@inl.gov>
@GiudGiud
Copy link
Contributor Author

GiudGiud commented Dec 7, 2023

Added block restriction to SUbdomainsDivision.
The true potential of all this will show in the transfers PR, where for the first time we will be able to match subdomains to subdomains in a Transfer. Or match subdomains to subapps. Both are pretty cool and only a special case of the divisions

@GiudGiud
Add tests for no-overlap between mesh divisions that are distributed …
a2e0d07 
…using positions

Add test for block restriction of SubdomainsDivision
@GiudGiud
Copy link
Contributor Author

GiudGiud commented Dec 7, 2023

Did all 4. Thanks for the suggestions! It definitely looks better to use the API that's cloest to the need

@GiudGiud GiudGiud merged commit a559f23 into idaholab:next Dec 8, 2023
42 of 44 checks passed
@GiudGiud GiudGiud deleted the PR_misc_meshdiv branch December 8, 2023 00:08
@GiudGiud
Copy link
Contributor Author

GiudGiud commented Dec 8, 2023

failure in NS is unrelated

thanks for the review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lindsayad lindsayad lindsayad approved these changes

Assignees

@GiudGiud GiudGiud

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@GiudGiud @moosebuild @lindsayad