sesame2spiner refactor by adamdempsey90 · Pull Request #641 · lanl/singularity-eos

adamdempsey90 · 2026-06-04T20:45:18Z

PR Summary

This does some refactoring on the code base to build sesame2spiner as a library that singularity-eos links in while maintaining the executable sesame2spiner. sesame2spiner was linking in singularity-eos for some things, so I have moved those shared headers into a new library called singularity-utils. For backwards compatibility in downstream codes, I have replaced the headers lost in singularity-eos/base with forwarding headers to singularity-utils. I have updated the includes in singularity-eos proper with the new ones.

If I did everything correctly, nothing should've changed. So my plan is to have this MR merged, then follow up with adding the functions to sesame2spiner the library to process tables without reading a file.

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

…for those

…es. Modify cmake so that we build a library.

adamdempsey90 · 2026-06-04T20:52:36Z

Evidently there's some cmake stuff left to figure it when things like spiner are not requested or when not in submodule mode maybe

Co-authored-by: Adam M. Dempsey <adamdemps@gmail.com>

adamdempsey90 · 2026-06-10T21:14:47Z

+  PRIVATE
+    sesame2spiner-lib


I'm not totally sure on if this should be private or public

I think for executables private is fine.

adamdempsey90 · 2026-06-10T21:18:31Z

This file is not included anywhere I think?

No it's not. We could add it to the singularity-eos whole test harness after this refactor perhaps? I think that can be a separate MR.

adamdempsey90 · 2026-06-10T21:20:26Z

Evidently there's some cmake stuff left to figure it when things like spiner are not requested or when not in submodule mode maybe

Okay, all tests are passing now so I think this is ready for review

buechlerm · 2026-06-11T12:13:59Z

Is there an expected lifecycle for the forwarding headers? If so, how should we communicate to users that the forwarding headers are in place? Our changelog mostly just points users to MR's, but maybe in this case something more concrete about changes they should make? Issue a warning if the forwarding header is used?

adamdempsey90 · 2026-06-11T13:30:03Z

Is there an expected lifecycle for the forwarding headers? If so, how should we communicate to users that the forwarding headers are in place? Our changelog mostly just points users to MR's, but maybe in this case something more concrete about changes they should make? Issue a warning if the forwarding header is used?

I see a few options:

I could add a #warning to those headers and then we delete them after they've been in place for 1 release. That could be communicated in the changelog for the next release.
I could just not have these at all and the next release is potentially breaking (I don't think that's a huge deal).
We always have the forwarding headers.

I don't really think people outside of us are going to act on the warnings, so I'm leaning towards option 2.

Yurlungur

I'm sorry for the long delay on this review. Some nitpicks below but they are all non-blocking.

Yurlungur · 2026-07-08T14:09:38Z


+# Install singularity-utils (required by singularity-eos_Interface)
+install(
+  TARGETS singularity-utils


does singularity-utils include the sesame2spiner executable? Having that go somewhere sensible upon install would be a nice improvement.

Yurlungur · 2026-07-08T14:12:15Z


 #include <eospac-wrapper/eospac_wrapper.hpp>
-#include <singularity-eos/eos/eos_spiner_construction.hpp>
+#include <singularity-utils/spiner_params.hpp>


I'm not sure how I feel about singularity-utils being outside the singularity-eos include folder... I think I like it.

Yurlungur · 2026-07-08T14:13:39Z

out of curiosity why two levels of sesame2spiner here? Just to make the include paths nice? Why not put it in singularity-utils/sesame2spiner?

Yurlungur · 2026-07-08T14:14:28Z


  // singularity version
-  H5LTset_attribute_string(file, "/", "singularity_version", SINGULARITY_VERSION);
+  H5LTset_attribute_string(file, "/", "singularity_version", SESAME2SPINER_VERSION);


Yurlungur · 2026-07-08T14:15:21Z

-#include <singularity-eos/base/spiner_table_utils.hpp>
+#include <singularity-utils/bounds.hpp>
+#include <singularity-utils/constants.hpp>
+#include <singularity-utils/fast-math/logs.hpp>


I question for the future, but I wonder if fast-math belongs in ports-of-call...

Yurlungur · 2026-07-08T14:24:42Z

 constexpr size_t MAX_NUM_LAMBDAS = 3;
 enum class DataStatus { Deallocated = 0, OnDevice = 1, OnHost = 2, UnManaged = 3 };
 enum class TableStatus { OnTable = 0, OffBottom = 1, OffTop = 2 };
-enum class TableSplit { Total = 0, ElectronOnly = 1, IonCold = 2 };


Yurlungur · 2026-07-08T14:26:28Z

+using table_utils::SpinerTableGridParams;
+using Bounds = table_utils::Bounds<table_utils::NGRIDS>;
+using Grid_t = Spiner::PiecewiseGrid1D<Real, table_utils::NGRIDS>;


I guess this one needs the using statements.

Yurlungur · 2026-07-08T14:28:19Z

For the future (not now) we could consider moving some of this into ports-of-call

Yurlungur · 2026-07-08T14:29:06Z

Could also consider moving this into ports-of-call... actually there may be duplicated functionality already...

Yurlungur · 2026-07-08T14:29:31Z

Another ports-of-call candidate (again for later)

adamdempsey90 added 8 commits June 3, 2026 12:43

Add a new singularity-utils library. This is the cmakelists file

c98bc16

Move some shared headers to a new shard library. Update the includes …

55d08b6

…for those

Move sesame2spiner files into a library and executable src/ directori…

63646a3

…es. Modify cmake so that we build a library.

Modify cmake and fix some include paths to get everything to compile

37f59e5

Add sesame2spiner namespace

5f041fd

Fix comment

9c90094

Add back forwarding headers for the ones we lost

ee628b5

Merge remote-tracking branch 'origin/main' into dempsey/sesrefac

4374103

adamdempsey90 mentioned this pull request Jun 5, 2026

Add a SpinerEosDependsRhoSie constructor that uses an existing eos object to generate a spiner eos #632

Merged

13 tasks

adamdempsey90 added 2 commits June 5, 2026 17:23

Updates for non submodule mode

a5c40a0

Use correct if checks

64cb334

adamdempsey90 commented Jun 5, 2026

View reviewed changes

Comment thread singularity-eos/eos/eos_gruneisen.hpp Outdated

adamdempsey90 commented Jun 5, 2026

View reviewed changes

Comment thread singularity-eos/eos/eos_vinet.hpp Outdated

adamdempsey90 commented Jun 5, 2026

View reviewed changes

Comment thread singularity-eos/eos/eos_vinet.hpp Outdated

adamdempsey90 and others added 6 commits June 5, 2026 17:49

ancient formatting

d17b292

Co-authored-by: Adam M. Dempsey <adamdemps@gmail.com>

Try this out

062f35d

Merge remote-tracking branch 'origin/main' into dempsey/sesrefac

81fdde2

Another attempt

02e42e2

Install sesame2spiner as a part of singularity-eos_INTERFACE

590ff79

Move up eigen finding

c492637

adamdempsey90 commented Jun 10, 2026

View reviewed changes

adamdempsey90 requested review from Yurlungur and buechlerm June 10, 2026 21:20

Move the spiner param struct and some helpers into singularity-utils

b6bfe26

Remove target sources

0fc0614

Yurlungur approved these changes Jul 8, 2026

View reviewed changes

Uh oh!

Conversation

adamdempsey90 commented Jun 4, 2026

PR Summary

PR Checklist

Uh oh!

adamdempsey90 commented Jun 4, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

adamdempsey90 commented Jun 10, 2026

Uh oh!

buechlerm commented Jun 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adamdempsey90 commented Jun 11, 2026

Uh oh!

Yurlungur left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

buechlerm commented Jun 11, 2026 •

edited

Loading