aspect
aspect copied to clipboard
fix invalid access to in.strain_rate
We set the size of MaterialModelInputs::strain_rate
to zero when it is not supplied, but it turns out that we ignore this fact and still access these entries. It seems that the compiler does not reliably detect this, #see 4862.
I see the following options:
- Always set to the correct size but fill with NaN. Problem: we check
in.strain_rate.size()
in a few places. - Provide an access function with an Assert inside and make
strain_rate
private. - Always fill the strain rate, especially considering that we have more situations where we do need the strain rate than we used to (it used to be only for viscosity)
@naliboff Does the original idea "only supply strain_rate if we need the viscosity" make any sense still?
@anne-glerum I am also curious what your thoughts about this are.
Hi @tjhei I think we could remove setting the size of the strain rate to 0 and make sure to request the minimum amount of material properties from the material model with in.requested_properties
as used in #4248. Setting the strain rate to size 0 was a flag for the material models to not compute the viscosity, and that task can be/is taken over by the requested properties.
Ok, but how do we detect if we need to fill strain_rate with real information? We need to ask the material model if any of the requested properties requires the strain rate to compute...
So there is ModelDependence
but it seems to be incomplete (no reaction rates, etc.) and unused so far. But that framework could be used to compute what inputs we need to fill based on the requested properties.
Ah I was thinking we just always fill the strain rate, but then in the material model decide to use it or not depending on the requested properties. Would it save a lot of time not to fill the strain rate?
I don't know how big the difference will be, but the motivation behind bool compute_strain_rate
was the desire to save computational time.
My main problem is that we need a minimal change for the 2.4 release now, without changing too much in the material model.
Ah I thought the time saving was avoiding computing the viscosity.
What if, for now, when we fill the MaterialModelInputs (line 271 and 356 of material_model/interface.cc), we don't just check
if (compute_strain_rate == false)
this->strain_rate.resize(0);
but also check for reaction_outputs?
if (compute_strain_rate == false && in.requested_properties != reaction_outputs)
this->strain_rate.resize(0);
We should check, but I don't think the other properties (apart from viscosity) need the strain rate.
@gassmoeller With your recent change to always compute the strain rate (#5268), can this be closed?
Yes, I think this was fixed by #5268.