Skip to content

Commit 608002d

Browse files
authored
Merge branch 'main' into buechler/spiner_constructor
2 parents ee9d2ff + 4997667 commit 608002d

12 files changed

Lines changed: 528 additions & 1494 deletions

.github/PULL_REQUEST_TEMPLATE.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ detail. Why is this change required? What problem does it solve?-->
1919
- [ ] Make sure the copyright notice on any files you modified is up to date.
2020
- [ ] After creating a pull request, note it in the CHANGELOG.md file.
2121
- [ ] LANL employees: make sure tests pass both on the github CI and on the Darwin CI
22+
- [ ] 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.
23+
- [ ] 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.
2224

2325
If preparing for a new release, in addition please check the following:
2426
- [ ] Update the version in cmake.

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
bin
2+
.codex
23
.spack*
34
spack.lock
45
utils/variant
@@ -14,3 +15,5 @@ CMakeUserPresets.json
1415
build/
1516
goldfiles.tar.gz
1617
.aider*
18+
CMakeCache.txt
19+
CMakeFiles

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
### Changed (changing behavior/API/variables/...)
1212

1313
### Infrastructure (changes irrelevant to downstream codes)
14+
- [[PR629]](https://github.com/lanl/singularity-eos/pull/629) Use macros and eos_base and eos_variant to reduce boiler plate
1415
- [[PR626]](https://github.com/lanl/singularity-eos/pull/626) Fix C++20 warnings related to lambdas
1516

1617
## Release 1.11.1

doc/sphinx/src/contributing.rst

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,33 @@ internal tests will not be run automatically. So when the code is
4646
ready for merge, you must ask a project maintainer to trigger the
4747
remaining tests for you.
4848

49+
AI-assisted coding
50+
-------------------
51+
52+
``singularity-eos`` requires that if AI was used to assist in code
53+
generation, a disclaimer must be made in a comment in the relevant
54+
file. Also if agentic AI was used, please have your agent dump a
55+
"proposed plan" markdown file in the ``plan_histories`` folder. This
56+
provides an LLM-readable history of machine-generated changes and
57+
helps disentangle human-made choices from machine-made ones. For
58+
example, if you used codex or claude code, use a workflow like this
59+
one:
60+
61+
1. Ask the agentic framework to propose a plan targeting your problem.
62+
2. Tell it to dump the plan into a new file in ``plan_histories``
63+
3. Iterate until you're happy with the code and submit an MR.
64+
4. After submitting the MR, rename the new file to be prefixed by the MR number and commit it.
65+
66+
If you submit code to ``singularity-eos`` you own that code and you are
67+
responsible for understanding it. If code is submitted that the author
68+
does not understand, the author will be asked to resubmit a changeset
69+
that they understand.
70+
71+
Finally, please be cognizant of reviewer time and effort. Agentic AI
72+
can create changesets much faster than a human can review them. When
73+
possible, please break up large changes and refactors into
74+
human-parse-able chunks.
75+
4976
Expectations for code review
5077
-----------------------------
5178

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
# Plan: Reduce `EosBase` Vector Wrapper Boilerplate with Macros
2+
3+
## Goal
4+
5+
Reduce duplication in `singularity-eos/eos/eos_base.hpp` for the CRTP-based vector API wrappers that call `portableFor`, without fundamentally changing the public API or overload set.
6+
7+
## Scope
8+
9+
Target the repeated vector wrapper patterns in `EosBase<CRTP>`:
10+
11+
- one-input / one-output wrappers
12+
- two-input / one-output wrappers
13+
- wrappers with a `scratch` forwarding overload
14+
- wrappers with a raw-pointer forwarding overload
15+
- the special vector wrapper that forwards to a scalar method with an output reference
16+
17+
Leave non-matching methods handwritten if the macro would become harder to read than the original code.
18+
19+
## Proposed Macro Shapes
20+
21+
### 1. `SG_EOS_VEC_2IN_1OUT`
22+
23+
Use for methods shaped like:
24+
25+
```cpp
26+
out[i] = copy.Method(in1[i], in2[i], lambdas[i]);
27+
```
28+
29+
Examples:
30+
31+
- `TemperatureFromDensityInternalEnergy`
32+
- `InternalEnergyFromDensityTemperature`
33+
- `PressureFromDensityTemperature`
34+
- `PressureFromDensityInternalEnergy`
35+
- `EntropyFromDensityTemperature`
36+
- `EntropyFromDensityInternalEnergy`
37+
- `SpecificHeatFromDensityTemperature`
38+
- `SpecificHeatFromDensityInternalEnergy`
39+
- `BulkModulusFromDensityTemperature`
40+
- `BulkModulusFromDensityInternalEnergy`
41+
- `GruneisenParamFromDensityTemperature`
42+
- `GruneisenParamFromDensityInternalEnergy`
43+
- `GibbsFreeEnergyFromDensityTemperature`
44+
- `GibbsFreeEnergyFromDensityInternalEnergy`
45+
46+
Suggested parameters:
47+
48+
- method name
49+
- first indexed input variable name
50+
- second indexed input variable name
51+
- output variable name
52+
- copy declaration kind: `const CRTP &` or `CRTP`
53+
54+
### 2. `SG_EOS_VEC_1IN_1OUT`
55+
56+
Use for methods shaped like:
57+
58+
```cpp
59+
out[i] = copy.Method(in1[i], lambdas[i]);
60+
```
61+
62+
Primary example:
63+
64+
- `MinInternalEnergyFromDensity`
65+
66+
Suggested parameters:
67+
68+
- method name
69+
- indexed input variable name
70+
- output variable name
71+
- copy declaration kind
72+
73+
### 3. `SG_EOS_VEC_2IN_REFOUT`
74+
75+
Use for methods shaped like:
76+
77+
```cpp
78+
copy.Method(in1[i], in2[i], out[i], lambdas[i]);
79+
```
80+
81+
Primary example:
82+
83+
- `InternalEnergyFromDensityPressure`
84+
85+
Suggested parameters:
86+
87+
- method name
88+
- first indexed input variable name
89+
- second indexed input variable name
90+
- output variable name
91+
- copy declaration kind
92+
93+
## Important Constraints
94+
95+
- Preserve the current overload names and signatures.
96+
- Preserve the `scratch` overload behavior.
97+
- Preserve the raw-pointer overload behavior with `Transform && = Transform()`.
98+
- Preserve current `copy` semantics:
99+
some methods use `const CRTP &copy`, while others use `CRTP copy`.
100+
- Keep `FillEos` handwritten unless a separate dedicated macro remains clearly readable.
101+
102+
## Suggested Implementation Order
103+
104+
1. Add the helper macros near the other EOS helper macros at the top of `eos_base.hpp`.
105+
2. Replace the repetitive vector wrapper bodies with macro invocations.
106+
3. Keep `FillEos` and any non-conforming methods handwritten.
107+
4. Build and run the existing test suite that covers EOS vector calls.
108+
5. Check compiler diagnostics carefully, since macro errors can be noisy in templated code.
109+
110+
## Expected Benefits
111+
112+
- substantially less boilerplate in `EosBase`
113+
- easier addition of new vector wrapper methods
114+
- lower risk of copy-paste inconsistencies between overloads
115+
- no fundamental change to the API seen by derived EOS classes
116+
117+
## Main Risks
118+
119+
- macro-generated template errors may be harder to read
120+
- overly generic macros can become less maintainable than the duplicated code
121+
- accidental mismatch between current `const CRTP &copy` and `CRTP copy` behavior
122+
123+
## Recommendation
124+
125+
Proceed with a small macro family rather than one fully generic macro. Three focused macros should remove most duplication while keeping the generated code readable and close to the current implementation.
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
# Plan: Reduce `Variant` Boilerplate in `eos_variant.hpp`
2+
3+
## Goal
4+
5+
Reduce duplication in `singularity-eos/eos/eos_variant.hpp` by introducing a small set of helper macros for the repeated `PortsOfCall::visit(...)` wrappers, without changing the public API of `Variant`.
6+
7+
## Summary
8+
9+
A macro-based cleanup should work well in `eos_variant.hpp`.
10+
11+
The vector section is especially repetitive and appears to be a better fit for macros than the already-refactored `EosBase` vector wrappers. Most methods repeat the same four-overload bundle:
12+
13+
1. a no-lambda overload that constructs `NullIndexer`
14+
2. a lambda-taking overload that dispatches through `PortsOfCall::visit`
15+
3. a `scratch` + no-lambda overload that constructs `NullIndexer`
16+
4. a `scratch` + lambda-taking overload that dispatches through `PortsOfCall::visit`
17+
18+
## Recommended Macro Families
19+
20+
### 1. Vector wrappers
21+
22+
Use a small family of macros for the repeated vector forwarding patterns:
23+
24+
- `SG_VARIANT_VEC_2IN_1OUT(NAME, IN1, IN2, OUT)`
25+
- `SG_VARIANT_VEC_1IN_1OUT(NAME, IN1, OUT)`
26+
- `SG_VARIANT_VEC_2IN_REFOUT(NAME, IN1, IN2, OUT)`
27+
28+
These would cover the methods that forward to the same method on the active EOS in the variant.
29+
30+
### 2. Optional scalar wrappers
31+
32+
If further cleanup is desired, a second macro family could reduce the scalar `PortsOfCall::visit(...)` wrappers:
33+
34+
- `SG_VARIANT_SCALAR_2IN_1OUT(NAME, A1, A2)`
35+
- `SG_VARIANT_SCALAR_1IN_1OUT(NAME, A1)`
36+
- `SG_VARIANT_SCALAR_2IN_REFOUT(NAME, A1, A2, OUT)`
37+
38+
This scalar cleanup is optional. The highest-value, lowest-risk target is the vector section.
39+
40+
## Best Targets For Vector Macros
41+
42+
These methods appear to fit the macro pattern well:
43+
44+
- `TemperatureFromDensityInternalEnergy`
45+
- `InternalEnergyFromDensityTemperature`
46+
- `PressureFromDensityTemperature`
47+
- `PressureFromDensityInternalEnergy`
48+
- `MinInternalEnergyFromDensity`
49+
- `EntropyFromDensityTemperature`
50+
- `EntropyFromDensityInternalEnergy`
51+
- `GibbsFreeEnergyFromDensityTemperature`
52+
- `GibbsFreeEnergyFromDensityInternalEnergy`
53+
- `SpecificHeatFromDensityTemperature`
54+
- `SpecificHeatFromDensityInternalEnergy`
55+
- `BulkModulusFromDensityTemperature`
56+
- `BulkModulusFromDensityInternalEnergy`
57+
- `GruneisenParamFromDensityTemperature`
58+
- `GruneisenParamFromDensityInternalEnergy`
59+
- `InternalEnergyFromDensityPressure`
60+
61+
## Methods To Leave Handwritten
62+
63+
These are less regular and should likely remain explicit:
64+
65+
- `FillEos`
66+
- `ReferenceDensityTemperature`
67+
- `ValuesAtReferenceState`
68+
- `DensityEnergyFromPressureTemperature`
69+
- various introspection helpers
70+
- serialization helpers
71+
72+
## Suggested Shape
73+
74+
The vector section could collapse to a short list of macro invocations such as:
75+
76+
```cpp
77+
SG_VARIANT_VEC_2IN_1OUT(TemperatureFromDensityInternalEnergy, rhos, sies, temperatures)
78+
SG_VARIANT_VEC_2IN_1OUT(InternalEnergyFromDensityTemperature, rhos, temperatures, sies)
79+
SG_VARIANT_VEC_2IN_1OUT(PressureFromDensityTemperature, rhos, temperatures, pressures)
80+
SG_VARIANT_VEC_2IN_1OUT(PressureFromDensityInternalEnergy, rhos, sies, pressures)
81+
SG_VARIANT_VEC_1IN_1OUT(MinInternalEnergyFromDensity, rhos, sies)
82+
SG_VARIANT_VEC_2IN_1OUT(EntropyFromDensityTemperature, rhos, temperatures, entropies)
83+
SG_VARIANT_VEC_2IN_1OUT(EntropyFromDensityInternalEnergy, rhos, sies, entropies)
84+
SG_VARIANT_VEC_2IN_1OUT(GibbsFreeEnergyFromDensityTemperature, rhos, temperatures, Gs)
85+
SG_VARIANT_VEC_2IN_1OUT(GibbsFreeEnergyFromDensityInternalEnergy, rhos, sies, Gs)
86+
SG_VARIANT_VEC_2IN_1OUT(SpecificHeatFromDensityTemperature, rhos, temperatures, cvs)
87+
SG_VARIANT_VEC_2IN_1OUT(SpecificHeatFromDensityInternalEnergy, rhos, sies, cvs)
88+
SG_VARIANT_VEC_2IN_1OUT(BulkModulusFromDensityTemperature, rhos, temperatures, bmods)
89+
SG_VARIANT_VEC_2IN_1OUT(BulkModulusFromDensityInternalEnergy, rhos, sies, bmods)
90+
SG_VARIANT_VEC_2IN_1OUT(GruneisenParamFromDensityTemperature, rhos, temperatures, gm1s)
91+
SG_VARIANT_VEC_2IN_1OUT(GruneisenParamFromDensityInternalEnergy, rhos, sies, gm1s)
92+
SG_VARIANT_VEC_2IN_REFOUT(InternalEnergyFromDensityPressure, rhos, Ps, sies)
93+
```
94+
95+
## Important Constraint
96+
97+
The macros in `eos_variant.hpp` will be slightly more delicate than those in `eos_base.hpp`, because they use perfect forwarding through `PortsOfCall::visit` lambdas. That is still manageable, but it argues for a small, focused macro family rather than an overly generic one.
98+
99+
## Recommendation
100+
101+
Proceed with the vector macro refactor first.
102+
103+
It offers the best reduction in boilerplate with limited risk and preserves the existing overload set. If the result reads well, consider a second pass for the scalar `visit(...)` wrappers.
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
# MR629 EOSPAC Vector Wrapper Macro Plan
2+
3+
## Goal
4+
5+
Reduce the boilerplate in `singularity-eos/eos/eos_eospac.hpp` for the repeated EOSPAC vector wrapper declarations that forward `2-in/1-out` calls through `EosBase<EOSPAC>`.
6+
7+
## Scope
8+
9+
Only target the repeated wrapper pattern with:
10+
11+
- two indexed real inputs
12+
- one indexed real output
13+
- the no-scratch overload
14+
- the scratch overload used for type-mismatch fallback
15+
16+
Do not introduce a macro for the warning strings themselves. Keep both warnings written verbatim inside the wrapper macro body.
17+
18+
Do not add a `1-in/1-out` macro. `MinInternalEnergyFromDensity` stays as-is.
19+
20+
Do not touch the specialized pointer-based EOSPAC vector implementations, `FillEos`, `DensityEnergyFromPressureTemperature`, or `ValuesAtReferenceState`.
21+
22+
## Proposed Macro
23+
24+
Add one EOSPAC-local macro near the vector wrapper declarations:
25+
26+
```cpp
27+
#define SG_EOSPAC_VEC_2IN_1OUT(NAME, IN1, IN2, OUT) \
28+
template <typename RealIndexer, typename ConstRealIndexer, typename LambdaIndexer> \
29+
inline void NAME(ConstRealIndexer &&IN1, ConstRealIndexer &&IN2, RealIndexer &&OUT, \
30+
const int num, LambdaIndexer &&lambdas) const { \
31+
PORTABLE_WARN("Not providing scratch memory will trigger scalar EOSPAC lookups"); \
32+
EosBase<EOSPAC>::NAME(std::forward<ConstRealIndexer>(IN1), \
33+
std::forward<ConstRealIndexer>(IN2), \
34+
std::forward<RealIndexer>(OUT), num, \
35+
std::forward<LambdaIndexer>(lambdas)); \
36+
} \
37+
template <typename RealIndexer, typename ConstRealIndexer, typename LambdaIndexer, \
38+
typename = std::enable_if_t<!is_raw_pointer<RealIndexer, Real>::value>> \
39+
inline void NAME(ConstRealIndexer &&IN1, ConstRealIndexer &&IN2, RealIndexer &&OUT, \
40+
Real * /*scratch*/, const int num, LambdaIndexer &&lambdas) const { \
41+
PORTABLE_WARN("EOSPAC type mismatch will cause significant performance degradation"); \
42+
EosBase<EOSPAC>::NAME(std::forward<ConstRealIndexer>(IN1), \
43+
std::forward<ConstRealIndexer>(IN2), \
44+
std::forward<RealIndexer>(OUT), num, \
45+
std::forward<LambdaIndexer>(lambdas)); \
46+
}
47+
```
48+
49+
## Planned Replacements
50+
51+
Use the macro for these wrapper declarations:
52+
53+
```cpp
54+
SG_EOSPAC_VEC_2IN_1OUT(TemperatureFromDensityInternalEnergy, rhos, sies, temperatures)
55+
SG_EOSPAC_VEC_2IN_1OUT(InternalEnergyFromDensityTemperature, rhos, temperatures, sies)
56+
SG_EOSPAC_VEC_2IN_1OUT(PressureFromDensityTemperature, rhos, temperatures, pressures)
57+
SG_EOSPAC_VEC_2IN_1OUT(PressureFromDensityInternalEnergy, rhos, sies, pressures)
58+
SG_EOSPAC_VEC_2IN_1OUT(EntropyFromDensityTemperature, rhos, temperatures, entropies)
59+
SG_EOSPAC_VEC_2IN_1OUT(EntropyFromDensityInternalEnergy, rhos, sies, entropies)
60+
SG_EOSPAC_VEC_2IN_1OUT(SpecificHeatFromDensityTemperature, rhos, temperatures, cvs)
61+
SG_EOSPAC_VEC_2IN_1OUT(SpecificHeatFromDensityInternalEnergy, rhos, sies, cvs)
62+
SG_EOSPAC_VEC_2IN_1OUT(BulkModulusFromDensityTemperature, rhos, temperatures, bmods)
63+
SG_EOSPAC_VEC_2IN_1OUT(BulkModulusFromDensityInternalEnergy, rhos, sies, bmods)
64+
SG_EOSPAC_VEC_2IN_1OUT(GruneisenParamFromDensityTemperature, rhos, temperatures, gm1s)
65+
SG_EOSPAC_VEC_2IN_1OUT(GruneisenParamFromDensityInternalEnergy, rhos, sies, gm1s)
66+
```
67+
68+
## Rationale
69+
70+
This follows the same general style as the vector API boilerplate macros in `singularity-eos/eos/eos_base.hpp`, but keeps the implementation local to EOSPAC because the warning behavior is EOSPAC-specific.
71+
72+
The result should remove most of the duplicated declarations without obscuring the actual fallback behavior.

0 commit comments

Comments
 (0)