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

Add general entropy functionality to EOS #233

Merged
merged 50 commits into from
Mar 16, 2023
Merged

Add general entropy functionality to EOS #233

merged 50 commits into from
Mar 16, 2023

Conversation

jhp-lanl
Copy link
Collaborator

@jhp-lanl jhp-lanl commented Feb 17, 2023

PR Summary

This MR adds the basic entropy hookups for entropy lookup functions to be exposed in the general EOS type.

There are five main changes in this effort:

  1. Add entropy lookups to the Variant class. This basically forces the EOS API to now provide entropy lookups so it needs to be implemented in some form everywhere.
  2. Add a default function to raise an error that entropy isn't implemented and have the entropy functions call this function by default.
  3. Use the ideal gas EOS as a prototype for adding entropy to other EOS. The functional form is fairly
  4. Incorporate entropy into the modifiers correctly. I could have just used the error functionality, but it was actually fairly easy since most modifiers won't modify the entropy. I had to prove to myself how entropy should be modified with scaling ratios, but I think it's the correct approach.
  5. Add a lot of documentation about entropy and complete vs incomplete EOS.

I wanted to implement entropy for at least one analytic EOS in this MR because I think it exposes important implications for other analytic EOS. Specifically, it demonstrates that the entropy is divergent at zero temperature for EOS with constant heat capacity, which further emphasizes the importance of the reference state in the entropy calculation.

There are a lot of changes to the documentation where I wanted to add what I've learned in researching how to implement entropy. There are also some cosmetic changes that can be ignored (especially in the Vinet EOS) that just
made things easier for me to read in text form.

To do:

  • Create a test for the ideal gas entropy

First step on addressing #210

PR Checklist

  • Adds a test for any bugs fixed. Adds tests for new features.
  • Format your changes by using the make format command after configuring with cmake.
  • Document any new features, update documentation for changes made.
  • N/A 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 If preparing for a new release, update the version in cmake.

@jhp-lanl
Copy link
Collaborator Author

@aematts can you review this?

Also @Yurlungur and @dholladay00 can you give things a brief look to make sure you agree with my strategy for defaulting EOS to throw errors?

@jhp-lanl jhp-lanl changed the title Add general entropy functionality to EOS WIP: Add general entropy functionality to EOS Feb 17, 2023
@jhp-lanl
Copy link
Collaborator Author

jhp-lanl commented Feb 17, 2023

I should add that I haven't built the code yet 😶 so there's probably lots of little issues I need to fix tomorrow. I just wanted to get this up so people could start looking at things.

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.

👍 This all seems reasonable to me.

.gitignore Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
doc/sphinx/src/models.rst Show resolved Hide resolved
doc/sphinx/src/models.rst Show resolved Hide resolved
singularity-eos/eos/eos_base.hpp Outdated Show resolved Hide resolved
singularity-eos/eos/eos_davis.hpp Outdated Show resolved Hide resolved
singularity-eos/eos/eos_eospac.hpp Outdated Show resolved Hide resolved
@jhp-lanl
Copy link
Collaborator Author

This MR depends on #234

Also add scratch functionality to `eos_base` and `eos_variant` entropy functions
@jhp-lanl
Copy link
Collaborator Author

@aematts any thoughts?

@dholladay00 I don't think there's much for you to review, but make sure you agree with how I'm implementing the default entropy error throwing

@jhp-lanl jhp-lanl changed the title WIP: Add general entropy functionality to EOS Add general entropy functionality to EOS Feb 28, 2023
@jhp-lanl
Copy link
Collaborator Author

@aematts any thoughts?

I should add, I'd greatly appreciate your eyes on the documentation, @aematts . I've written some stuff up about EOS theory that mirrors the document you wrote (which I cited). I'd love your feedback there if you can give it.

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.

This looks good. Thanks for doing this, @jhp-lanl ! A high priority should be enabling entropy in the tabulated EOS's but getting the framework in place is an important first step.

doc/sphinx/src/models.rst Outdated Show resolved Hide resolved
singularity-eos/eos/eos_base.hpp Outdated Show resolved Hide resolved
singularity-eos/eos/eos_base.hpp Show resolved Hide resolved
@Yurlungur Yurlungur added the enhancement New feature or request label Mar 1, 2023
@aematts
Copy link
Collaborator

aematts commented Mar 2, 2023

@aematts any thoughts?

I should add, I'd greatly appreciate your eyes on the documentation, @aematts . I've written some stuff up about EOS theory that mirrors the document you wrote (which I cited). I'd love your feedback there if you can give it.

I have been to a conference this week and have had no time to even read my emails. I will get to this sometime next week.

@jhp-lanl
Copy link
Collaborator Author

jhp-lanl commented Mar 2, 2023

@aematts any thoughts?

I should add, I'd greatly appreciate your eyes on the documentation, @aematts . I've written some stuff up about EOS theory that mirrors the document you wrote (which I cited). I'd love your feedback there if you can give it.

I have been to a conference this week and have had no time to even read my emails. I will get to this sometime next week.

@aematts I'm going to delay until the end of the day Monday to merge this, but even if you don't have time to make comments by then, please feel free to make comments even after these changes are merged. It will then be fairly easy to just create a new PR to address your comments on these changes, especially if most of the comments are in the documentation.

Normally I'd just wait for all the feedback to come in, but given that this MR has been open for a while already without feedback and that Jonah has a student waiting for these changes, I figure it's possible to merge these changes with the expectation that more changes will be required down the road. Besides, I'll need to create another MR to add in EOSPAC, spiner, and Davis entropy capabilities.

@dholladay00
Copy link
Collaborator

@jhp-lanl what's the timeline for merging this?

@jhp-lanl
Copy link
Collaborator Author

@dholladay00 sorry last week was pretty crazy. I was going to merge it last week but forgot about it. I'll try to resolve the conflicts in the changelog this afternoon and get it merged.

@aematts feel free to give feedback after the MR has been merged and I can address your feedback in subsequent MRs

@jhp-lanl
Copy link
Collaborator Author

I'll try to resolve the conflicts in the changelog this afternoon and get it merged.

I'll make this a top priority for tomorrow

@jhp-lanl
Copy link
Collaborator Author

Merging... all CI is green

@jhp-lanl jhp-lanl merged commit 60d85f5 into main Mar 16, 2023
@jhp-lanl jhp-lanl deleted the jhp/entropy branch March 16, 2023 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants