Skip to content

Make sieMin/sieMax pass through systemunits - #658

Merged
adamdempsey90 merged 10 commits into
mainfrom
buechler/table_bounds
Sep 23, 2026
Merged

adamdempsey90 merged 10 commits into
mainfrom
buechler/table_bounds

Conversation

@buechlerm

@buechlerm buechlerm commented Sep 21, 2026 •

Copy link
Copy Markdown
Collaborator

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.
  • 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.
  • LANL employees: make sure tests pass both on the github CI and on the Darwin CI
  • If ML was used, make sure to add a disclaimer at the top of a file indicating ML was used to assist in generating the file.
  • If Agentic AI was used, have the AI generate a "proposed changes" markdown file and store it in the plan_histories folder, with a filename the same as the MR number.

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.
  • Maintainers: ensure spackages are up to date:
    • LANL-internal team, update XCAP spackages
    • Current maintainer of upstream spackages, submit MR to spack

@buechlerm buechlerm self-assigned this Sep 21, 2026

@Yurlungur Yurlungur left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm happy to merge this now. But we should consider adding sieMin and sieMax to the general EOS API. We could add a base implementation that just returns sensible bounds... Probably sie(rho=rho_at_Pmin, T=0) and +infinity and we can override.

Comment on lines +295 to +300
// Pass-throughs for the table bounds accessors some models provide,
// e.g., SpinerEOS and StellarCollapse. These are not part of the
// EosBase contract, so they are only well formed when the underlying
// EOS provides them. Since UnitSystem is a template, these bodies are
// only instantiated when called, so UnitSystem<T> remains valid for a
// T without these accessors until one of them is actually used.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm not sure how I feel about this being implicitly defined like this. I think it's quite confusing even with the comment.

In particular, the error message from a failed compilation will be very confusing for a new developer. Perhaps instead, we could use an if constexpr here that checks if the function exists and otherwise static assert?

Comment thread plan_histories/MR658-2026-09-21-modifier-energy-bounds.md
Comment thread singularity-eos/eos/modifiers/scaled_eos.hpp Outdated
Comment thread singularity-eos/eos/eos_base.hpp
Comment thread singularity-eos/eos/eos_spiner_rho_temp.hpp
Comment thread singularity-eos/eos/eos_spiner_rho_temp.hpp Outdated
Co-authored-by: Jonah Miller <jonah.maxwell.miller@gmail.com>
Comment thread doc/sphinx/src/models.rst
Comment thread singularity-eos/eos/eos_base.hpp
Comment thread singularity-eos/eos/eos_eospac.hpp Outdated
Comment thread test/test_eos_modifiers.cpp
@Yurlungur

Copy link
Copy Markdown
Collaborator

@buechlerm from my perspective this is good to go.

Comment thread doc/sphinx/src/models.rst
Comment thread test/test_eos_modifiers.cpp
@adamdempsey90
adamdempsey90 merged commit de6c0ff into main Sep 23, 2026
10 checks passed
@adamdempsey90
adamdempsey90 deleted the buechler/table_bounds branch September 23, 2026 14:12
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