Jmm/indexable types

[Yurlungur]

PR Summary

While integrating 3T into riot, I began to encounter scenarios where the lambda machinery of singularity-eos starts to become burdensome. In particular, the z-split modifier on top of (for example) a SpinerEOS means that both the base/underlying EOS and the modifier require lambda arguments. This is fine in principle; modifier lambda arguments get "appended" on to the back of the lambda and the code traverses through it like a stack. However, given that type is erased, it can be difficult for a host code to keep track of what lambda argument goes where, or even if a lambda argument is needed.

This MR resolves this issue by the introduction of IndexableTypes which may be used in indexers for lambdas. Put simply, an IndexableType is an empty struct that can be used to name an index, like an enum, but more free-form. If you overload the operator[] function for a lambda indexer to accept an IndexableType, most EOS classes that expect IndexableTypes will be able to interact with it by name, rather than position in the lambda. For example, if you define:

class MyLambda_t {
 public:
  MyLambda_t() = default;
  PORTABLE_FORCEINLINE_FUNCTION
  Real &operator[](const std::size_t idx) {
    return data_[idx];
  }
  PORTABLE_FORCEINLINE_FUNCTION
  Real &operator[](const MeanIonizationState &zbar) {
    return data_[2];
  }
 private:
  std::array<Real, 3> data_;
};

then you would be able to interact with it as

MyLambda_t lambda;
lambda[MeanIonizationState()] = zbar;
Real P = eos.PressureFromDensityTemperature(rho, T, lambda);

integer and named indexes are interchange-able and backwards compatible, and I provide the ability to create new named types for custom EOS's as well as a convenience struct that automatically builds an indexer form a variadic list of named types. This machinery is all in the singularity-eos/base/indexable_types.hpp header.

I also introduce the function NeedsLambda, which is now exposed in the variant. This function takes an indexable type and returns true if that indexable type is needed by the class for a given lambda and false otherwise.

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

[Yurlungur]

I also revise the PTE solver to now actually access the lambda indexer, unless it is passed a NullIndexer in which case it will use the chache.

[Yurlungur]

One thing we may wish to consider down the line: should we be backwards compatible with integer indexing in lambdas? Or should we eventually remove that optionality? Currently, that's used in the xrage code get_sg_*. But it could potentially be removed by adding an appropriately overloaded cache object. Probably a discussion for another time. The integer-based indexing plays nicer with the fortran host codes.

[jhp-lanl]

One thing we may wish to consider down the line: should we be backwards compatible with integer indexing in lambdas? Or should we eventually remove that optionality? Currently, that's used in the xrage code get_sg_*. But it could potentially be removed by adding an appropriately overloaded cache object. Probably a discussion for another time. The integer-based indexing plays nicer with the fortran host codes.

(I'm reviewing right now) My opinion is that we should by default use a named approach within our code. I'd have to think more about what would be best for the vector interface though.

[Yurlungur]

(I'm reviewing right now) My opinion is that we should by default use a named approach within our code. I'd have to think more about what would be best for the vector interface though.

Yeah agreed. This isn't something we need to change right now. But if we break backwards compatibility some of the template/sfinae complexity I implement here can go away. I'm reluctant to completely remove it right now though, as the backwards compatibility gives us time to transition and test how it feels and where it might break.

[Yurlungur]

Tests now passing on re-git.

[jhp-lanl]

I think it's worth considering breaking out the square bracket invocable check from the base class. I think that would simplify some of this code significantly.

[jhp-lanl]

The logic here confuses me a bit. I fee like if the NullIndexer is provided, it should signal that no lambda information is given. Whereas I feel like here you're trying to communicate that the lambda information is in the Cache array.

What if instead you had a default template argument for LambdaIndexer and set that to something other than the NullIndexer? The basic idea I'm exploring is that I feel like there's a place to provide a NullIndexer when either all EOS don't care about lambda information or that information is optional.

[Yurlungur]

The idea here is that if the lambda indexer is passed in as a null indexer, we still need a cache---we can't pass in nullptr. That's because the PTE solver may be caching log density and log temperature for root finds for SpinerEOS. That's what this mechanism is for.

That said, maybe the code base has moved beyond needing this, as I think everywhere where we call the PTE solver, we manage the cache ourselves. If we are happy with always managing the cache ourselves, then this can be removed. We can just always call the lambda indexer.

[Yurlungur]

I am going to remove this special logic and assume that the lambda is always manually managed externally. get_sg_* and friends already do this, so long as the size of the scratch array is properly computed. I think it's actually long past time we remove this weird internal logic where the PTE solver manages its own cache. (Which we can blame on me... I added it in the first place as a hack.)

[Yurlungur]

Thanks for the review @jhp-lanl ! I think all your comments should be resolved now.

[dholladay00]

Does this work if I nest modifiers each of which requires lambdas?