Make getUNodal function in ConservativeAdvection

[tophmatthews]

Closes #31633

[moosebuild]

Job Documentation, step Docs: sync website on de34b6c wanted to post the following:

View the site here

This comment will be updated on new commits.

[moosebuild]

Job Coverage, step Generate coverage on de34b6c wanted to post the following:

Framework coverage

	236997	#31634 de34b6
	Total	Total	+/-	New
Rate	85.93%	85.93%	-0.00%	100.00%
Hits	122919	122919	-	2
Misses	20122	20123	+1	0

Diff coverage report

Full coverage report

Modules coverage

Coverage did not change

Full coverage reports

Reports

• framework
• chemical_reactions
• combined
• contact
• electromagnetics
• external_petsc_solver
• fluid_properties
• fsi
• functional_expansion_tools
• geochemistry
• heat_transfer
• level_set
• misc
• navier_stokes
• optimization
• peridynamics
• phase_field
• porous_flow
• ray_tracing
• rdg
• reactor
• richards
• scalar_transport
• solid_mechanics
• solid_properties
• stochastic_tools
• subchannel
• thermal_hydraulics
• xfem

This comment will be updated on new commits.

[loganharbour]

Suggested change

	virtual GenericReal<is_ad> getUNodal(unsigned int n) { return _u_nodal[n]; }
	virtual const GenericReal<is_ad> & getUNodal(const std::size_t n) { return _u_nodal[n]; }

AD containers are big - we definitely don't want to copy them

[tophmatthews]

Ready for re-review

[tophmatthews]

wait...I don't want to return a ref, since downstream I want something like this,

  virtual const GenericReal<is_ad> & getUNodal(const std::size_t n) override
  {
    return std::exp(this->_u_nodal[n]);
  }

and this leads to warning: returning reference to local temporary object [-Wreturn-stack-address]

[tophmatthews]

Okay, if I turn the _upwinding into a bool instead of an enum, which is valid as far as I can tell since it's either FULL or NONE, then I can now override computeResidual and computeJacobian to fill a container that holds the exponent of _u_nodal:

template <bool is_ad>
void
LogConservativeAdvectionTempl<is_ad>::computeJacobian()
{
  if (this->_upwinding)
    for (const auto n : make_range(_exp_u_nodal.size()))
      _exp_u_nodal[n] = protectedExp(this->_u_nodal[n]);
  ConservativeAdvectionTempl<is_ad>::computeJacobian();
}

which can then be called via getUNodal, e.g.,

  virtual const GenericReal<is_ad> & getUNodal(const std::size_t n) override
  {
    return _exp_u_nodal[n];
  }

[tophmatthews]

Ready for re-review @loganharbour @lindsayad

[lindsayad]

Okay, if I turn the _upwinding into a bool instead of an enum

You should still be able to make comparisons with an enum. I prefer the enum because it allows the opportunity for expanding the list of enumerators in the future. Full upwinding is only first order accurate. There's also second order upwinding and many other types of upwind schemes one can construct

[tophmatthews]

oh I thought I couldn't inherit enums. I'll try again...

[tophmatthews]

indeed, I was thinking of something different. fixed it now. Thanks @lindsayad !

[lindsayad]

Just the one suggestion