-
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
Hyperelastic RMT code with updated modular precision #767
base: master
Are you sure you want to change the base?
Conversation
…ngruent with the HLLC 5-equation formulation
…need phase change
@ChrisZYJ I merged your PR if you could try to help get those features back into this one, it would be highly appreciated! |
I can first help merge that PR. @mrodrig6 Could you please give me the access to edit? Thanks! |
I’ve identified the issue. In the original code, hypoelastic calculations defaulted to 4th-order finite differencing regardless of input file parameters. However, the updated code adjusts the order based on the fd_order parameter. Initially, fd_order was intended solely as a post-processing parameter for vorticity and numerical Schlieren calculations. It didn't affect the simulations, as stated in the documentation and case file comments. In the #771 case file, fd_order was set to 1. With the change in default behavior, the simulation now uses first-order finite differencing, which is less stable and led to the observed stability issues. @sbryngelson, could you please revert my previous PR? The simulation results have always been correct, and the discrepancy came from the change in default behavior of hypoelastic calculations. Sorry for my oversight. I can update the documentation later to clarify the role of fd_order. |
I created a PR for this: #776 @ChrisZYJ can you look it over to be sure it's correct and then approve it and I will merge. Thanks! |
@ChrisZYJ Yes and done. I responded earlier this week to this comment. I agree with @sbryngelson that this is resolved |
I agree with this causing the issue and good to know. |
So if I understand correctly, this PR is waiting on tests and then it will be ready for code review? (of course I can review it earlier as well) |
@sbryngelson Of course! Happy to help |
@mrodrig6 I can help make a couple of minor changes. Also, if you'd like to convert an example into a test suite, I can do that - please just let me know which examples would cover all the added code. Sorry that your invitation has expired; could you please invite me to your repository again? Thanks! Also, I see you removed the checker that prohibits hypoelasticity with HLLC. Do both hypoelasticity and hyperelasticity now work with both HLL and HLLC? |
Type of change
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.
Tested the code on NCSA Delta and passed the full test suite and most example cases.
Provide instructions so we can reproduce.
Ran on 2 GPUs with one CPU each on an interactive node
./mfc.sh test -j 8 --max-attempt 3 -- -c delta
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.