Ignored boundary temperature model in ASPECT

[bobmyhill]

Currently, if the user does not set "Fixed temperature boundary indicators" in their "Boundary temperature model", ASPECT ignores the model completely. This is implicit in the description of the "Fixed temperature boundary indicators" parameter the manual, but isn't obvious from looking through the tests.

Doing a quick grep reveals that there are 733 tests that include a "Boundary temperature model" section, but only 574 of these set the "Fixed temperature boundary indicators" variable. That implies that 159 tests don't actually use the model that is listed in the file.

Should we change
if (fixed_temperature_boundary_indicators.size() == 0)
{
model_names.clear();
model_operators.clear();
}

to
if (fixed_temperature_boundary_indicators.size() == 0)
{
AssertThrow(model_names.size() == 0, <some useful message here>);
}

?

Other possibilities:

• Remove all unused models from the tests.
• Add a slightly longer description to the "Fixed temperature boundary indicators" parameter.

[gassmoeller]

My first reaction was: I would like to be able to set a boundary model but not actually have any prescribed boundaries, because it allows to quickly switch the boundary model on/off by only adding the indicator. But then I had to think about how often that is a use case (and how much effort it is to comment one additional parameter) vs. how often users set a boundary temperature model, but dont specify the fixed boundary indicators and only afterwards realize that they do not set a boundary temperature after all. The fact that I trained myself to always check both parameters when changing something about the boundary temperature probably means we should be more strict and use the AssertThrow. So from my point of view go ahead with the change (but also check the model_operators.size()).

Remove all unused models from the tests.

Yes, please! And all unused boundary temperature sections! Tedious but appreciated work!

Add a slightly longer description to the "Fixed temperature boundary indicators" parameter.

More documentation is (almost) always better. But I think the AssertThrow change above will be the safest, not everyone reads the documentation of every parameter ;-).

[bobmyhill]

Hi @gassmoeller, great, thank you for your thoughts, I'm glad you agree.
I'll make these changes now.

[bobmyhill]

Addressed by #4162

[gassmoeller]

#4162 was merged, so I will close this.

[bangerth]

Thanks indeed -- boring work that nevertheless makes the whole project better!