EOSPAC performance could be hit significantly when passing the wrong types

[jhp-lanl]

I just noticed in the EOSPAC class that we still use the base class vector overloads:
https://github.com/lanl/singularity-eos/blob/35a42e2dc4adc70f29d10a90aea57e31055646a5/singularity-eos/eos/eos_eospac.hpp#LL85C3-L85C3

What I just realized though is that this means that the EOSPAC performance in singularity-eos will be very sensitive to the types passed to the vector interface. If the user passes a generic type that accepts a square bracket operator, the template resolution will call the base class vector interface which will slowly loop over states in the container and send individual values to EOSPAC. Of course you'll get a type error if the contained type is incompatible in some way, but most containers shouldn't run into this issue.

If my logic is correct, then a user will see a significant performance hit when using the wrong types. Is a better solution to maybe provide EOSPAC its own templated overloads that will alert the user to the type mismatch? Maybe something like this?

template <typename RealIndexer, typename ConstRealIndexer, typename LambdaIndexer>
  inline void
  TemperatureFromDensityInternalEnergy(ConstRealIndexer &&rhos, ConstRealIndexer &&sies,
                                       RealIndexer &&temperatures, const int num,
                                       LambdaIndexer &&lambdas) const {
     PORTABLE_WARN("EOSPAC type mismatch will cause significant performance degradation");
     using EosBase<EOSPAC>::TemperatureFromDensityInternalEnergy;
     TemperatureFromDensityInternalEnergy(rhos, sies, temperatures, num, lambdas);

I assume this has to be done at runtime since the determination of which type is actually contained in the variant is a runtime decision, right? So in other words I'd think we can't use neat compile time tricks.

Alternatively, this could be an abort situation if we consider performance important enough.

@jonahm-LANL @dholladay00 and @rbberger what are your thoughts here?

Let me know if I'm completely off-base, but my experience with the singularity-strength work has showed me that a generic templated path is required if EOSPAC is contained in the variant since all variant types need to support generic vector types.

[dholladay00]

This is a good point. That is a decent solution, another option is error out entirely. This is a decent solution b/c it allows them to do something ... highly suboptimal... but it will be very painful with lots of warning output.

[jhp-lanl]

This is a good point. That is a decent solution, another option is error out entirely. This is a decent solution b/c it allows them to do something ... highly suboptimal... but it will be very painful with lots of warning output.

Yeah an error may be preferable for the reasons you give. The warning won't print for every scalar lookup at least so the spew won't be too intense at least. I'm open to either possibility.

EDIT - Also the above uses PORTABLE_WARN rather than PORTABLE_ALWAYS_WARN so these would only be debug warnings.

[Yurlungur]

I like your solution @jhp-lanl I think it actually can be done at compile time, though I'm not sure that's desirable. This template resolution should only happen if the eospac pointer overload isn't called, which is known at compile time. So we could do a static_assert if we really want it to fail to compile.

I think either a static assert or an always warn would be the way to go. This should fail loudly to avoid performance regressions. But I'm not sure it should completely not work.

[jhp-lanl]

This template resolution should only happen if the eospac pointer overload isn't called, which is known at compile time. So we could do a static_assert if we really want it to fail to compile.

Ah is this because template resolution happens first? So will the compiler resolve the templates first and then perform overload resolution to figure out which versions get called before it has to evaluate any static asserts? So in other words, the static assert won't even be compiled if the correct types are used?

I'd prefer the static assert route if it works. Let me play with that and then I'll make an MR here to include that.

[Yurlungur]

Yeah that's right. Which versions of the code are called have to be known at compile time for template resolution to work.