-
Notifications
You must be signed in to change notification settings - Fork 100
Fix chemistry‑related bugs #829
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
base: master
Are you sure you want to change the base?
Conversation
has the code that automatically turns a 1D case into an extrapolated 2D one been put into MFC? @DimAdam-01 |
I consider it a distinct feature that I intended to add separately, but I can include it in this pull request. @sbryngelson |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #829 +/- ##
=======================================
Coverage 43.15% 43.16%
=======================================
Files 68 68
Lines 20262 20263 +1
Branches 2424 2424
=======================================
+ Hits 8745 8746 +1
Misses 10054 10054
Partials 1463 1463 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
"cantera==3.0.1", | ||
"pyrometheus==1.0.2" | ||
"cantera==3.1.0", | ||
"pyrometheus@git+https://github.com/DimAdam-01/pyrometheus.git@main" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can't make master MFC use your fork of pyrometheus....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The original version encountered build errors on Frontier, so I introduced a few modifications to verify whether it would compile successfully there, and it did.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
then we need to create a PR for the Pyrometheus fix first
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I’m speaking with Esteban at the moment.
Description
resolve the issue that prevented multi‑rank execution when using non‑hardcoded initial conditions;
and fix minor bugs in both the inline Riemann solver and the HLL Riemann solver.
Type of change
Scope
How Has This Been Tested?
Run the 1D shocktube reaction case, compared the result and they had the exact match,
Test Configuration:
Run on Delta and Delta AI ( A100 and GH GPUs), as well as on my personal laptop
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.