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

Implement general EOS equations in Riemann Solver #338

Merged
merged 36 commits into from
Aug 2, 2023

Conversation

psharda
Copy link
Contributor

@psharda psharda commented Jul 31, 2023

Resolves #337 and #271 and #278
Includes #339

src/HLLC.hpp Outdated Show resolved Hide resolved
@psharda
Copy link
Contributor Author

psharda commented Jul 31, 2023

@BenWibking I implemented equation A.5a of Kershaw+1998 to find $C^{*}_{\rho}$ for arbitrary EOS. I have kept $C^{*}_{P}$ the same as before (so, it is only applicable for ideal gas EOS). Now, for gamma_law, dedr = 0 in microphysics. So, equation A.5a of Kershaw reduces to: $C^{*}_{\rho} = \frac{1}{2} \left(I^{-} + I^{+} \right)$. However, since both $I^{-}$ and $I^{+}$ are > 0 for several Hydro test problems, I don't obtain $C^{*}_{\rho} = 0$ as we'd expect for ideal gas. Why?

All the tests (except for HydroSMS) pass anyway, which I don't understand. Is the issue here that equation A5.a is an approximate equation, or that I changed $C^{*}_{\rho}$ but not $C^{*}_P$?

@psharda
Copy link
Contributor Author

psharda commented Jul 31, 2023

Ugh, why are my $$ not working above to display inline equations

@BenWibking
Copy link
Collaborator

BenWibking commented Jul 31, 2023

All the tests (except for HydroSMS) pass anyway, which I don't understand. Is the issue here that equation A5.a is an approximate equation, or that I changed $C_{\rho}$ but not $C_P$?

The issue is the A.5a is approximate.

For reasons I don't fully understand (but have seen in experimenting with different ways to calculate the wavespeeds for ideal gases), the HydroSMS test is very sensitive to the wavespeed estimates. So I don't think this is anything to worry about.

@BenWibking
Copy link
Collaborator

Hmm. It does look bad though:
hydro_sms_1 0000

It doesn't get much better unless I add a huge amount of artificial viscosity, which is not something I'd want to do in a real simulation.

Let's hope it's less oscillatory once you use the numerical approximation for $C_P^{\star}$.

@psharda
Copy link
Contributor Author

psharda commented Jul 31, 2023

hydro_sms_1.0000.pdf
The oscillations seem less, but the spike is larger.

src/EOS.hpp Show resolved Hide resolved
@psharda
Copy link
Contributor Author

psharda commented Jul 31, 2023

This is how it was before:

image

@BenWibking
Copy link
Collaborator

@BenWibking does Microphysics also have (∂2P∂2ρ)s

Not sure. You'll have to ask Mike.

@psharda
Copy link
Contributor Author

psharda commented Aug 2, 2023

hmm, I pushed some more commits but they haven't appeared here.

@BenWibking
Copy link
Collaborator

Wow, it looks basically perfect:
hydro_sms_1 0000

@psharda
Copy link
Contributor Author

psharda commented Aug 2, 2023

Wow, it looks basically perfect: hydro_sms_1 0000

Woah, didn't expect that. It's even better than what we had before this PR.

@psharda
Copy link
Contributor Author

psharda commented Aug 2, 2023

/azp run

@psharda
Copy link
Contributor Author

psharda commented Aug 2, 2023

all tests will pass now. any more comments @BenWibking ?

@psharda psharda requested a review from BenWibking August 2, 2023 17:04
@BenWibking
Copy link
Collaborator

As long as the GPU tests pass, it looks good to me.

@BenWibking BenWibking added this pull request to the merge queue Aug 2, 2023
Merged via the queue into quokka-astro:development with commit 685110f Aug 2, 2023
8 checks passed
@psharda psharda deleted the riemann-microp branch August 3, 2023 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

compute Roe-average sound speed for a general EOS
2 participants