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

Make Adami loop flipping optimization optional #581

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

efaulhaber
Copy link
Member

Depends on #498.

I now get an error of 0.0 for the dam break validation between a single threads and 128 threads on an AMD Threadripper 3990X. So we can generate all validation files on as many threads as we have available.

Copy link

codecov bot commented Jul 24, 2024

Codecov Report

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

Project coverage is 68.50%. Comparing base (f051b55) to head (10df125).
Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
...chemes/boundary/dummy_particles/dummy_particles.jl 57.14% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #581      +/-   ##
==========================================
- Coverage   68.51%   68.50%   -0.02%     
==========================================
  Files          87       87              
  Lines        5406     5407       +1     
==========================================
  Hits         3704     3704              
- Misses       1702     1703       +1     
Flag Coverage Δ
unit 68.50% <57.14%> (-0.02%) ⬇️

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

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

@efaulhaber
Copy link
Member Author

@svchb do you remember what we did with this? Did we implement it somewhere else? Or did we just forget about it?

@svchb
Copy link
Collaborator

svchb commented Dec 12, 2024

The optimization is there because there are cases where the larger system is looped over and there are no/only a small number of neighbors between systems. In my simulations this made up to a 20% difference in run time.

One side effect is though that if you run the validation cases with different numbers of threads the results change.

@efaulhaber
Copy link
Member Author

Yes, I know. This PR is based on your PR, so the diff is messy. But did we make the flipping optional and disable it for validiation? Or did we forget about it? Or did we decide not to do this for some reason?

@efaulhaber
Copy link
Member Author

Just rebased to make the diff useful.

@svchb
Copy link
Collaborator

svchb commented Dec 12, 2024

No this was not merged. You also did not create all the validation files correctly. I think this was the latest on this PR.

@efaulhaber efaulhaber added the enhancement New feature or request label Jan 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants