-
Notifications
You must be signed in to change notification settings - Fork 76
fix low density/Pressure SCVH EOS tables #997
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: main
Are you sure you want to change the base?
Changes from all commits
b282ad2
aba2d31
51b48f0
11c6859
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -248,7 +248,7 @@ subroutine read_namelist | |||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| ! set defaults | ||||||||||||||||||||||||||||||||||||||||||||
| mass_list = 'mass_frac.txt' | ||||||||||||||||||||||||||||||||||||||||||||
| table_version = 51 | ||||||||||||||||||||||||||||||||||||||||||||
| table_version = 52 | ||||||||||||||||||||||||||||||||||||||||||||
| eos_version = 1 | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| !set T range | ||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -564,7 +564,12 @@ subroutine mesa_eos_eval( logRho0, logT, mass_Frac, eos_result) | |||||||||||||||||||||||||||||||||||||||||||
| end if | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| eos_result(1:num_eos_basic_results) = res | ||||||||||||||||||||||||||||||||||||||||||||
| eos_result(i_lnRho) = logRho | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| ! FreeEOS data files store these logarithms in base 10. | ||||||||||||||||||||||||||||||||||||||||||||
| eos_result(i_lnPgas) = res(i_lnPgas)/ln10 | ||||||||||||||||||||||||||||||||||||||||||||
| eos_result(i_lnE) = res(i_lnE)/ln10 | ||||||||||||||||||||||||||||||||||||||||||||
| eos_result(i_lnS) = res(i_lnS)/ln10 | ||||||||||||||||||||||||||||||||||||||||||||
| eos_result(i_lnRho) = log10Rho | ||||||||||||||||||||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great catch! I think you're right about this. Do you know if it has an impact on anything? In practice, I think we cut out most of the region where FreeEOS was failing and falling back to MESA with the "logQ_cut"-related boundary controls. Anyway, I've always been a bit uncomfortable with the idea that our "FreeEOS" tables that were meant to replace the older EOS might end up falling back to the older EOS without being very clear about that. So I like the fix, but I don't really like the existence of the thing that we're fixing here...
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've definitely seen models crash from this, in fact this issue from jaime comes to mind : #822
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. after asking an ai to check this further: It affects every row marked total numeric rows: 14,399,221 examples pulled from the tables comparing the old and new free eos tables: Here are a few matched v51/v52 fallback rows from
| |
||||||||||||||||||||||||||||||||||||||||||||
| eos_result(i_dpe) = 0._dp | ||||||||||||||||||||||||||||||||||||||||||||
| eos_result(i_dsp) = 0._dp | ||||||||||||||||||||||||||||||||||||||||||||
| eos_result(i_dse) = 0._dp | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
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.
Why did the write format change?
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.
Some values overflowed the old fixed-width
f10.5columns when I regenerated the eosDT tables as part of this PR. I still need to check why this overflow happened. It might be symptoms of another bug i might have introduced in this pr, so i'm looking closer.