Analytic electron EOS and zsplit modifier #444

Yurlungur · 2024-12-11T03:00:13Z

PR Summary

In this MR I implement an analytic electron EOS---this is essentially an ideal gas but with a Gruneisen coefficient set by the the degrees of freedom of an electron and a specific heat that scales with ionization state divided by mean atomic mass. The idea here is that you pass in ion density and electron temperature. The ionization state is also passed in through the lambda.

I am also in the process of implementing the z split as a modifier, which splits an equation of state into a component from the ions and a component from the electrons. The z split will be two modifiers, one EOS for electrons and one for ions. Both applied to a single EOS split it into two. z split is exact for an ideal gas but not for anything else, but it is commonly used. I'm not done with this yet, so it might not be in the diff yet, depending on when you look at the MR.

Note that in the case of nuclear burning, EOS's will also need to be potentially modified by a mass scale, as per our ScaledEOS modifier.

I will cover adding electron or ion sub-tables of tabulated Sesame EOS's in a subsequent MR.

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 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.
Ensure that any when='@main' dependencies are updated to the release version in the package.py

…mentation

…plit and IdealElectron modles

Yurlungur

@jhp-lanl @jdolence @pdmullen @chadmeyer this is now ready for review.

Yurlungur · 2024-12-17T16:28:51Z

.github/workflows/formatting.yml

@@ -9,7 +9,7 @@ on:
 jobs:
    formatting:
      name: Check Formatting
-      runs-on: ubuntu-latest
+      runs-on: ubuntu-noble


Latest is now 24.04 which does not support clang-format-12. For now, just pin to older Ubuntu. WIll resolve the dependency issue long term in a formal single MR in the new year. See #446

this is now ubuntu-22.04. Must use version number, not version name.

Yurlungur · 2024-12-17T16:29:24Z

CMakeLists.txt

+      # jmm: updated Dec 17, 2024 to avoid build errors on modern gcc
+      GIT_TAG v3.7.1)


Catch2 v3.0.1 does not compile on ubuntu 24.04. Update to 3.7.1 to rescue it.

Yurlungur · 2024-12-17T16:30:52Z

CMakeLists.txt

+         # Suppresses annoying ABI notes. See:
+         # https://stackoverflow.com/questions/52020305/what-exactly-does-gccs-wpsabi-option-do-what-are-the-implications-of-supressi
+         $<$<CXX_COMPILER_ID:GNU>:-Wno-psabi>


GCC outputs these ABI notes. When compiling with -Werr those notes cause compilation to fail, but the notes explain gcc bugs, and are not singularity-eos problems. In the way we build singularity-eos (via spack or in-tree), ABI incompatibilities will never impact us.

Yurlungur · 2024-12-17T16:31:40Z

singularity-eos/eos/eos_base.hpp

+// JMM: Use VA_ARGS to capture more complex template types
+#define SG_ADD_BASE_CLASS_USINGS(...)                                                    \


The problem here is that preprocessor macros interpret commas as separate arguments. This allows us to pass classes with multiple template arguments through the macro transparently.

Nice addition!

Yurlungur · 2024-12-17T16:32:07Z

singularity-eos/eos/eos_base.hpp

-  }                                                                                      \
+  }
+
+#define SG_ADD_MODIFIER_MEAN_METHODS(t_)                                                 \


split these out into separate modifier methods in preparation for isotopics.

Yurlungur · 2024-12-17T16:35:55Z

singularity-eos/eos/modifiers/zsplit_eos.hpp

+  template <typename Indexer_t = Real *>
+  PORTABLE_FORCEINLINE_FUNCTION Real GetScale_(Indexer_t &&lambda) const {
+    Real Z = GetIonizationState_(lambda);
+    if constexpr (ztype == ZSplitComponent::Electrons) {


Notice the if constexpr here, which makes this a compile-time switch rather than runtime. We could make it runtime, but I chose not to. The trade-off here is that run-time would make the variant smaller. But compile time increases performance.

…ng time, for unclear reasons

Yurlungur · 2024-12-17T22:25:32Z

Github updated the latest ubuntu runner to 24.04. This breaks everything. Actions taken:

For formatting, I pinned the runner to an older one. We'll need to resolve a formatting sweep later.
For other tests, I massaged build system or resolved caught warnings to keep things building.
For whatever reason gcc builds are much slower on 24.04 than previously. Builds are timing out. I have taken the following actions:
- github runners have 4 threads. Now all github tests use make -j4
- I shrank the default variant, removing some analytic equations of state. I would also like to remove the relativistic modifier from the variant by default, but it's too threaded through the python API. A problem for the future.
- If that's not enough, I will disable Python builds from the github build.

…bols

Yurlungur · 2024-12-17T23:45:52Z

OK in addition to the timeout, the full fortran interface was running out of memory in the linker phase. Additional actions taken:

For github full test build, switch to -DCMAKE_BUILD_TYPE=Release which drops debug info
Remove user memory limits in the docker image
Switch to the ld.gold linker, which is faster and more memory efficient.

Yurlungur · 2024-12-18T00:00:41Z

.github/workflows/docs.yml

@@ -40,7 +40,7 @@ jobs:
                  -DSINGULARITY_USE_SPINER_WITH_HDF5=OFF \
                  -DSINGULARITY_FORCE_SUBMODULE_MODE=ON \
                  -B build .
-            cmake --build build
+            cmake --build build --parallel 4


builds the python bits faster. Speeds up doc build time by a factor of 4ish

Yurlungur · 2024-12-18T00:00:53Z

.github/workflows/formatting.yml

@@ -9,7 +9,7 @@ on:
 jobs:
    formatting:
      name: Check Formatting
-      runs-on: ubuntu-latest
+      runs-on: ubuntu-22.04


Needed until we update clang-format to clang-17

Yurlungur · 2024-12-18T00:01:28Z

.github/workflows/tests.yml

Changes in this file required to deal with out-of-memory failure when linking the full fortran interface.

Yurlungur · 2024-12-18T00:03:38Z

.github/workflows/tests.yml

@@ -21,13 +21,15 @@ jobs:
        - name: install dependencies
          run: |
            sudo apt-get update -y -qq
-            sudo apt-get install -y --allow-downgrades --allow-remove-essential --allow-change-held-packages -qq build-essential gfortran libhdf5-serial-dev
+            sudo apt-get install -y --allow-downgrades --allow-remove-essential --allow-change-held-packages -qq build-essential gfortran libhdf5-serial-dev binutils-gold


binutils-gold gets us use the gold linker developed by google specifically for big C++ projects. It seems to be much faster and more memory efficient. Generically I don't know if we want to rely on it, but honestly I was very impressed.

I'll defer to @rbberger @dholladay00 and @mauneyc-LANL on their opinions here

Yeah i think for now we don't need to change it elsewhere. In this particular CI test, this, GNU ld is too memory inefficient and the test doesn't finish without gold.

I could totally be wrong, but I have a bit of a hunch that among all the changes made, only the switch to Release is required. Should we be minimalistic here, or you are quite happy with binutils-gold?

I was actually impressed by binutils-gold. The build time is also just faster with it on, so I'd like to leave it as this test takes about 10 minutes. That said, I'm fine removing it if (a) that still works and (b) people feel strongly.

Yurlungur · 2024-12-18T00:03:53Z

.github/workflows/tests.yml

+            ulimit -m unlimited
+            ulimit -v unlimited


Not sure if this does anything inside a docker container, but it doesn't hurt.

But we could also be minimalistic if desirable... see above. No strong feelings here.

I'll leave it. In worst case it does nothing.

Yurlungur · 2024-12-18T00:05:38Z

.github/workflows/tests.yml

                  -DSINGULARITY_PLUGINS=$(pwd)/../example/plugin \
+                  -DCMAKE_LINKER=ld.gold \
+                  -DCMAKE_BUILD_TYPE=Release \


Compared to the default build type, which is RelWithDebInfo, this drops debug symbols, shrinking the memory footprint during link stage.

I'm surprised that's such a huge memory hit to be honest. Honestly though, I find that if I really need debug symbols, I'm compiling with full Debug anyway so I'm okay with this change.

Yeah. I agree. Is also only for this test. The default build is still "RelWithDebInfo."

Yurlungur · 2024-12-18T00:05:51Z

.github/workflows/tests.yml

                  ..
                  #-DSINGULARITY_TEST_PYTHON=ON \
                  #-DSINGULARITY_TEST_STELLAR_COLLAPSE=ON \
                  #..
-            make
+            make -j4


parallel build.

Yurlungur · 2024-12-18T00:08:28Z

CMakeLists.txt

+         # `-Wclass-memaccess now default with -Wall but we explicitly
+         # manage this ourselves in our serialization routines.
+         $<$<AND:$<CXX_COMPILER_ID:GNU>,$<COMPILE_LANGUAGE:CXX>>:-Wno-class-memaccess>


This is apparently a new warning. Since we sometimes build with -Werr its also an error. I am silencing it as memcpy is an intrinsic part of the serialization infrastructure and the heuristic that normally justifies warning doesn't apply in our case. We have introspection about the trivial and non-trivial copy properties of our data and we manage it.

Yurlungur · 2024-12-18T00:12:38Z

spack-repo/packages/singularity-eos/package.py

ping @rbberger just so you're aware

Yurlungur · 2024-12-18T00:15:07Z

spack-repo/packages/singularity-eos/package.py

Ping @rbberger so you're aware

Yurlungur · 2024-12-18T00:15:54Z

Tests now passing on github and re-git.

doc/sphinx/src/models.rst

swjones · 2024-12-18T15:36:14Z

Looks good to me, tests added. The applicability of the Z-split is still not clear to me but that's something we will work on separately.

pdmullen

LGTM! Non-blocking comments.

pdmullen · 2024-12-18T16:35:41Z

.github/workflows/tests.yml

@@ -21,13 +21,15 @@ jobs:
        - name: install dependencies
          run: |
            sudo apt-get update -y -qq
-            sudo apt-get install -y --allow-downgrades --allow-remove-essential --allow-change-held-packages -qq build-essential gfortran libhdf5-serial-dev
+            sudo apt-get install -y --allow-downgrades --allow-remove-essential --allow-change-held-packages -qq build-essential gfortran libhdf5-serial-dev binutils-gold


I could totally be wrong, but I have a bit of a hunch that among all the changes made, only the switch to Release is required. Should we be minimalistic here, or you are quite happy with binutils-gold?

pdmullen · 2024-12-18T16:36:07Z

.github/workflows/tests.yml

+            ulimit -m unlimited
+            ulimit -v unlimited


But we could also be minimalistic if desirable... see above. No strong feelings here.

singularity-eos/eos/modifiers/zsplit_eos.hpp

jhp-lanl

Minor changes

doc/sphinx/src/models.rst

doc/sphinx/src/using-eos.rst

jhp-lanl · 2024-12-18T17:18:31Z

singularity-eos/eos/eos_base.hpp

+// JMM: Use VA_ARGS to capture more complex template types
+#define SG_ADD_BASE_CLASS_USINGS(...)                                                    \


Nice addition!

singularity-eos/eos/modifiers/zsplit_eos.hpp

Co-authored-by: Jeff Peterson <[email protected]>

…/zsplit

Yurlungur · 2024-12-18T19:53:01Z

Thanks @jhp-lanl and @pdmullen for the reviews! I think all comments should be addressed (as best they currently can with available physics fidelity). Regarding thermodynamic derivatives... I wonder if we can do better down the line. Something to consider once we have access to an ionization model.

jhp-lanl · 2024-12-18T19:57:23Z

Regarding thermodynamic derivatives... I wonder if we can do better down the line. Something to consider once we have access to an ionization model.

I completely agree. Not something to tackle now, but definitely something to keep in mind

Yurlungur · 2024-12-18T21:22:04Z

Thanks all! Merged. 🎊

jonahm-LANL added 7 commits December 10, 2024 19:50

ideal gas electron EOS implemented. Still need to add zsplit and docu…

2b258e4

…mentation

zsplit mostly implemented

6167b75

zsplit compiles

4b4a2fa

tests work

3d18cae

add test for zsplits

7fba185

messages

2512249

Add discussion of lambdas, add discussion of 3T, add discussion of ZS…

8085e61

…plit and IdealElectron modles

Yurlungur changed the title ~~WIP: Analytic electron EOS and zsplit modifier~~ Analytic electron EOS and zsplit modifier Dec 17, 2024

jonahm-LANL and others added 6 commits December 17, 2024 09:09

changelog

7e9a7ec

Merge branch 'main' into jmm/zsplit

54c8a25

clang format 12 stay around please

aaa57cc

latest for docs, noble for formatting

0dfef59

try updating to latest catch2 release

a0a8069

unused variable

c692221

Yurlungur commented Dec 17, 2024

View reviewed changes

jonahm-LANL added 2 commits December 17, 2024 10:03

typo and also try to make wno-psabi only for C++

98fb444

try to fix cuda errors for static members

08172b2

Yurlungur requested review from jhp-lanl, chadmeyer, pdmullen and AstroBarker December 17, 2024 17:15

jonahm-LANL added 6 commits December 17, 2024 10:21

No GCC. You are wrong and I am right.

95c60cd

disable some EOSs from default variant

3b030b1

shrink the default variant

7886d11

Disable python build in github tests as it now takes an incredibly lo…

dc803ed

…ng time, for unclear reasons

update build to be parallel

a0f00c4

try enabling python build now again with parallel builds

c43afcc

jonahm-LANL added 2 commits December 17, 2024 15:29

undo removal of relativistic EOS

d282d09

fix type list comma

090559e

jonahm-LANL added 4 commits December 17, 2024 16:14

switch to ld.gold linker

39990f8

huh...

6f6b6f8

disabling intraprocedural optimization seems to way slow down the build

830bb5e

extend memory limits on system, use release build to remove debug sym…

d0dcacd

…bols

jonahm-LANL added 5 commits December 17, 2024 16:50

runner not available for ubuntu-noble now. Try older version string?

bc721f8

add doc info

fc3ee77

add v&v EOS option to spackage

0e9f910

spackage should have vandv set to true

2c013c3

document spackage change

7368490

Yurlungur commented Dec 18, 2024

View reviewed changes

Yurlungur linked an issue Dec 18, 2024 that may be closed by this pull request

Smaller default variant #445

Closed

This was referenced Dec 18, 2024

3T roadmap #440

Open

update spackage #431

Open

swjones reviewed Dec 18, 2024

View reviewed changes

doc/sphinx/src/models.rst Show resolved Hide resolved

pdmullen approved these changes Dec 18, 2024

View reviewed changes

jhp-lanl requested changes Dec 18, 2024

View reviewed changes

jonahm-LANL and others added 5 commits December 18, 2024 12:01

Sam jones comments

4e93e12

Update doc/sphinx/src/modifiers.rst

551fc3e

Co-authored-by: Jeff Peterson <[email protected]>

Update doc/sphinx/src/modifiers.rst

6e255b9

Co-authored-by: Jeff Peterson <[email protected]>

hjhp and pdmullen comments

5042643

Merge branch 'jmm/zsplit' of github.com:lanl/singularity-eos into jmm…

03d646d

…/zsplit

jhp-lanl approved these changes Dec 18, 2024

View reviewed changes

Yurlungur merged commit 70dfc73 into main Dec 18, 2024
8 checks passed

Yurlungur deleted the jmm/zsplit branch December 18, 2024 21:21

		# jmm: updated Dec 17, 2024 to avoid build errors on modern gcc
		GIT_TAG v3.7.1)

		// JMM: Use VA_ARGS to capture more complex template types
		#define SG_ADD_BASE_CLASS_USINGS(...) \

Analytic electron EOS and zsplit modifier #444

Analytic electron EOS and zsplit modifier #444

Conversation

Yurlungur commented Dec 11, 2024 • edited Loading

PR Summary

PR Checklist

Yurlungur left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yurlungur commented Dec 17, 2024 • edited Loading

Yurlungur commented Dec 17, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yurlungur commented Dec 18, 2024

swjones commented Dec 18, 2024

pdmullen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jhp-lanl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yurlungur commented Dec 18, 2024

jhp-lanl commented Dec 18, 2024

Yurlungur commented Dec 18, 2024

Yurlungur commented Dec 11, 2024 •

edited

Loading

Yurlungur commented Dec 17, 2024 •

edited

Loading