-
Notifications
You must be signed in to change notification settings - Fork 70
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
Update on adaptive time stepping for sub-grid bubbles #408
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #408 +/- ##
==========================================
+ Coverage 58.37% 58.41% +0.04%
==========================================
Files 57 57
Lines 14440 14453 +13
Branches 1892 1892
==========================================
+ Hits 8429 8443 +14
Misses 5449 5449
+ Partials 562 561 -1 ☔ View full report in Codecov by Sentry. |
Is this ready for review @lee-hyeoksu? |
If so, can you add an example in |
@sbryngelson This feature already has an example case Do you have any idea why Benchmark fails on Phoenix? It seems like it's beed canceled due to time limit but I couldn't find details.
Georgia Tech | Phoenix (NVHPC) (cpu) |
There's a problem with Phoenix at the moment. I'm sure it will be fixed shortly. |
Hi @lee-hyeoksu -- everything seems in order. There is a problem with Frontier. Perhaps @wilfonba or @anandrdbz can fix it. I also messaged them on Slack. |
@lee-hyeoksu. The only oddity I spotted was that these lines (https://github.com/lee-hyeoksu/MFC-Caltech/blob/7d571f1496444e51d6fcf1fd99032bdeb46686e0/src/simulation/m_time_steppers.fpp#L275-L280) are duplicates of these lines (https://github.com/lee-hyeoksu/MFC-Caltech/blob/7d571f1496444e51d6fcf1fd99032bdeb46686e0/src/simulation/m_time_steppers.fpp#L176-L181). I'm not 100% sure this is the problem, but allocating memory with Cray compilers can be finicky. |
@wilfonba Yeah It seems like a redundancy so I removed one of them. Let's see if now it works. Thank you for finding it. |
@sbryngelson @wilfonba Frontier test failed with exit code 135. Do you have any ideas? |
I see. I'm rerunning it. It didn't finish all the 3D jobs, not sure why. |
Co-authored-by: Hyeoksu Lee <[email protected]>
Description
This PR includes some updates on adaptive time stepping for sub-grid bubbles to improve the accuracy of the scheme. Also, I tried to make the subroutine
s_compute_bubble_source
be more concise by modularizing some parts into a few separate subroutines.This PR basically does not fix any bug nor introduce new features.
Type of change
Please delete options that are not relevant.
Scope
If you cannot check the above box, please split your PR into multiple PRs that each have a common goal.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes.
Provide instructions so we can reproduce.
Please also list any relevant details for your test configuration
Test Configuration:
Checklist
docs/
)examples/
that demonstrate my new feature performing as expected.They run to completion and demonstrate "interesting physics"
./mfc.sh format
before committing my codeIf your code changes any code source files (anything in
src/simulation
)To make sure the code is performing as expected on GPU devices, I have:
nvtx
ranges so that they can be identified in profiles./mfc.sh run XXXX --gpu -t simulation --nsys
, and have attached the output file (.nsys-rep
) and plain text results to this PR./mfc.sh run XXXX --gpu -t simulation --omniperf
, and have attached the output file and plain text results to this PR.