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

Use amu instead of 1/n_A for m_nucleon in gamma_law #1240

Merged
merged 11 commits into from
Jul 21, 2023

Conversation

psharda
Copy link
Collaborator

@psharda psharda commented Jun 28, 2023

Is this what we'd want to do (for gamma_law)?

@zingale
Copy link
Member

zingale commented Jun 28, 2023

I'm fine with this, but I want to run it though our suites, since there will be a lot of small changes. But I think it is a good idea to sync up with the standard constants here.

@psharda
Copy link
Collaborator Author

psharda commented Jun 28, 2023

I'm fine with this, but I want to run it though our suites, since there will be a lot of small changes. But I think it is a good idea to sync up with the standard constants here.

Sounds good. We may also need to update the reference solution for the test that fails.

@zingale
Copy link
Member

zingale commented Jul 4, 2023

@BenWibking
Copy link
Collaborator

This looks good to me. @zingale do we need to update the reference solution for the eos_cell test? If so, how do we do that?

@zingale
Copy link
Member

zingale commented Jul 5, 2023

sorry, I will do that. I'll try to get this done today and do a PR to your PR

@zingale
Copy link
Member

zingale commented Jul 6, 2023

There's a PR to @psharda 's PR, that once merged will fix the EOS test. Then this is good to merge.

@BenWibking
Copy link
Collaborator

There's a PR to @psharda 's PR, that once merged will fix the EOS test. Then this is good to merge.

@zingale just wanted to check- did this PR get merged?

@zingale
Copy link
Member

zingale commented Jul 17, 2023

@psharda said that you were trying to understand something before merging this
is it all good now?

update eos_cell unit test benchmark
@psharda
Copy link
Collaborator Author

psharda commented Jul 18, 2023

This PR should pass all tests now, and will be ready for review.

Copy link
Collaborator

@BenWibking BenWibking left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@BenWibking
Copy link
Collaborator

@zingale are you happy with this in its current form? It's ready to go on our end.

@zingale zingale merged commit 5a7415f into AMReX-Astro:development Jul 21, 2023
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.

3 participants