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

Compute thermodynamic derivatives and implement dens+pres mode in primordial chem EOS #1302

Merged
merged 10 commits into from
Aug 4, 2023

Conversation

psharda
Copy link
Collaborator

@psharda psharda commented Jul 27, 2023

This PR adds the eos_input_rp mode for primordial chem EOS. It also computes the sound speed and stores it in state.cs if pressure is defined.

… to first derivative of sqrt (sound speed) w/r/t dens
@psharda psharda changed the title Compute sound speed and implement dens+pres mode in primordial chem EOS Compute thermodynamic derivatives and implement dens+pres mode in primordial chem EOS Aug 2, 2023
BenWibking
BenWibking previously approved these changes Aug 2, 2023
@psharda psharda dismissed BenWibking’s stale review August 2, 2023 18:32

added one more commit

@psharda psharda requested a review from BenWibking August 2, 2023 18:32
@BenWibking
Copy link
Collaborator

@zingale @maxpkatz are you all happy with this? We can hold off on merging until you get a change to review if you'd like.

@@ -27,6 +27,7 @@ struct eos_t:eos_base_t {
amrex::Real dsdr{};
amrex::Real dpde{};
amrex::Real dpdr_e{};
amrex::Real G{}; // fundamental derivative (Thompson 1971), we use equation 2.24 of Menikoff & Plohr 1989
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary for this to go in the eos_base_t?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But this is in eos_t and not eos_base_t. We use eos_t for primordial chem and chem_eos_t for other Quokka problems.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@maxpkatz Is there a better place we can put it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry my comment above should have been less ambiguous: my concern was about adding a new quantity to eos_t which only some of the equations of state fill in. Part of the motivation for creating chem_eos_t was so that Quokka could modify that without affecting any of the other equations of state. So in my ideal world, we would only add this G field to chem_eos_t, and then add a has_G function analogous to has_pressure that wraps around the filling of this field in the EOS, so that the other applications could continue to use the gamma_law EOS without picking up a new quantity. But I may have misunderstood and didn't realize that Quokka was using eos_t in addition to chem_eos_t.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We still use eos_t for the primordial_chem network, since we need the species fractions to compute the EOS quantities. I suppose we could add the species fraction array to chem_eos_t, but my recollection of the rationale for this was that we didn't want to include that when just using the gamma_law EOS.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, @maxpkatz created chem_eos_t so that we can use Microphysics EOS for Quokka problems that have no species.

@maxpkatz maxpkatz merged commit 2ebca13 into AMReX-Astro:development Aug 4, 2023
21 checks passed
@zingale
Copy link
Member

zingale commented Aug 6, 2023

catching up on this -- we should add G to all of the other EOSes if it is going to be in the eos_t

@BenWibking
Copy link
Collaborator

That would be useful for us, so we can run the test problems in your EOS paper. Is it straightforward to do for the tabulated EOSes?

@maxpkatz
Copy link
Member

maxpkatz commented Aug 6, 2023

Not at present, we don't have the second derivatives of pressure in Helmholtz. However, this is something we would likely have to add if we were to re-implement the electron-positron portion of Helmholtz from the Timmes EOS, so we could revisit it then.

zingale pushed a commit to AMReX-Astro/Castro that referenced this pull request Aug 9, 2023
maxpkatz pushed a commit to maxpkatz/Microphysics that referenced this pull request Aug 9, 2023
…mordial chem EOS (AMReX-Astro#1302)

Add the eos_input_rp mode for primordial chem EOS. Also, compute the sound speed and stores it in state.cs if pressure is defined.
@psharda psharda deleted the compute-cs branch September 23, 2023 13:18
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.

4 participants