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

Update closure model documentation #358

Merged
merged 20 commits into from
Apr 16, 2024
Merged

Update closure model documentation #358

merged 20 commits into from
Apr 16, 2024

Conversation

jhp-lanl
Copy link
Collaborator

@jhp-lanl jhp-lanl commented Apr 12, 2024

PR Summary

Updates the PTE solver documentation. Here is a summary of the additions/changes

  • Various minor notation adjustments to make things more clear
  • Reorganized things into a general discussion of the governing equations (separate from PTE) and the $\bar{\rho}$ formulation and the implication that volume fractions and densities are both inputs and outputs.
  • Reformulated the constraint equations to better reflect what's in the code. Reorganized the density-energy/density-temperature discussion to reflect the changes.
  • Added descriptions for the pressure/temperature known versions

PR Checklist

  • N/A Adds a test for any bugs fixed. Adds tests for new features.
  • N/A Format your changes by using the make format command after configuring with cmake.
  • Document any new features, update documentation for changes made.
  • Make sure the copyright notice on any files you modified is up to date.
  • After creating a pull request, note it in the CHANGELOG.md file.
  • N/A LANL employees: make sure tests pass both on the github CI and on the Darwin CI

If preparing for a new release, in addition please check the following:

  • Update the version in cmake.
  • Move the changes in the CHANGELOG.md file under a new header for the new release, and reset the categories.

@jhp-lanl jhp-lanl marked this pull request as draft April 12, 2024 21:41
@jhp-lanl jhp-lanl requested review from Yurlungur and aematts April 12, 2024 21:41
@jhp-lanl
Copy link
Collaborator Author

See https://lanl.github.io/singularity-eos/jhp/docs/index.html to view my changes

@jhp-lanl jhp-lanl marked this pull request as ready for review April 12, 2024 22:43
@jhp-lanl
Copy link
Collaborator Author

@Yurlungur and @aematts I think I have everything updated. Please review

Copy link
Collaborator

@Yurlungur Yurlungur left a comment

Choose a reason for hiding this comment

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

Thanks for adding this more detailed documentation, @jhp-lanl !

doc/sphinx/src/using-closures.rst Outdated Show resolved Hide resolved
doc/sphinx/src/using-closures.rst Outdated Show resolved Hide resolved
@Yurlungur
Copy link
Collaborator

@aematts how does this look to you? If you're happy I'll merge.

@aematts
Copy link
Collaborator

aematts commented Apr 16, 2024

I am looking at it now. Sorry about holding you up but I was on my Frday off and took my husband to the neurologist Monday.

@Yurlungur
Copy link
Collaborator

I am looking at it now. Sorry about holding you up but I was on my Frday off and took my husband to the neurologist Monday.

Not at all--there's no rush!

@aematts
Copy link
Collaborator

aematts commented Apr 16, 2024

OK @jhp-lanl and @Yurlungur. I have gone through it once now. Your turn.

@aematts
Copy link
Collaborator

aematts commented Apr 16, 2024

One more thing: We might want to point out that if you use sesame tables they should NOT be inverted if you use the pressure-temperature formalism, but they SHOULD BE inverted if you use the density-energy formalism.

@aematts
Copy link
Collaborator

aematts commented Apr 16, 2024

It is all about what gives you the best derivatives.

@aematts
Copy link
Collaborator

aematts commented Apr 16, 2024

Fixed pressure should also be done on original tables (NOT inverted).
Fixed temperature doesn't matter but using the original tables would probably be better, avoiding inversion when it is not needed.

@Yurlungur
Copy link
Collaborator

One more thing: We might want to point out that if you use sesame tables they should NOT be inverted if you use the pressure-temperature formalism, but they SHOULD BE inverted if you use the density-energy formalism.

These are good rules of thumb to recommend, though I'm sure there are exceptions that prove the rule.

@jhp-lanl
Copy link
Collaborator Author

One more thing: We might want to point out that if you use sesame tables they should NOT be inverted if you use the pressure-temperature formalism, but they SHOULD BE inverted if you use the density-energy formalism.

I have a note in there about the need to invert from density-temperature space to density-energy space for the density-energy formulation. I mentioned that there are both performance and accuracy implications (I'm not sure how much I trust the inverted derivatives for example).

@jhp-lanl
Copy link
Collaborator Author

I'll merge once the docs build and I see they look okay. Thanks for the review @aematts and @Yurlungur !

Copy link
Collaborator

@aematts aematts 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 now.

@jhp-lanl jhp-lanl merged commit 91253bc into main Apr 16, 2024
4 checks passed
@jhp-lanl jhp-lanl deleted the jhp/docs branch April 16, 2024 23:51
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