Found while reading how porosity enters J2Erosion compared to the new Permafrost cap model. No test or production deck in the tree is affected today, so this is a trap for the next deck rather than a live bug. Reporting rather than fixing, because the choice among the options below is Elyce's.
The three defaults disagree
| Consumer |
Line |
Default |
ACEThermalParameters |
src/LCM/evaluators/ACEThermalParameters_Def.hpp:506,550 |
0.60 |
Permafrost |
src/LCM/models/Permafrost_Def.hpp:176 |
0.0 |
J2Erosion |
src/LCM/models/J2Erosion_Def.hpp:19 |
0.0 |
In Permafrost the 0.0 is a deliberate sentinel and is safe. Porosity there does exactly one thing, override the crushable volume W, and the override is guarded:
if (porosity > 0.0) P.W = porosity; // Permafrost_Def.hpp:596
Omit the key and the Frozen Parameters / Thawed Parameters values of W simply stand. Nothing silently changes.
In J2Erosion there is no such guard. Porosity is not a parameter with a fallback, it is the second coordinate of three empirical surface fits, and 0.0 flows straight into them as a real porosity value:
std::tie(ne, ny, nk) = unit_fit(ice_saturation, porosity); // J2Erosion_Def.hpp:571
ScalarT E = elastic_modulus_(cell, pt) * ne;
ScalarT Y = yield_strength_(cell, pt) * ny;
ScalarT K = hardening_modulus_(cell, pt) * nk;
What porosity 0 does to the fits
All three fits are bilinear in (ice saturation x, porosity y) and dominated by the cross term x*y. Set y = 0 and the cross term vanishes, leaving only negative contributions:
| porosity |
ice sat |
E mult |
Y mult |
K mult |
| 0.0 |
0.0 |
-0.0411 |
0.0000 |
-0.0462 |
| 0.0 |
0.5 |
-0.1806 |
-0.0344 |
-0.2277 |
| 0.0 |
1.0 |
-0.3202 |
-0.0688 |
-0.4091 |
| 0.6 |
1.0 |
0.4719 |
0.5725 |
0.4364 |
E_fit_max is in fact negative for every ice saturation whenever porosity is below about 0.205, and increasing the ice saturation makes it more negative, not less.
The clamps on lines 577-581 then catch it:
E = std::max(E, residual_elastic_modulus_);
Y = std::max(Y, soil_yield_strength_);
so the material does not blow up. It pins to ACE Residual Elastic Modulus and ACE Soil Yield Strength at every point, at every step, at every ice saturation. That is the fully thawed state. A deck that forgets the key gets a J2 Erosion material with no ice strength at all, with no error, no warning, and a run that completes normally and looks plausible.
The failure is quiet in the worst way: ACE_Ice_Saturation is still computed and still written to the output, so the Exodus file shows ice present and the mechanics behaving as if there were none.
Current exposure
Every deck that actually instantiates Model Name: J2 Erosion sets a porosity key, so nothing in the tree is wrong right now:
tests/LCM/ACE/MiniErosion/materials_mechanical_denudation.yaml:34
tests/LCM/ACE/MiniErosionClean/materials_mechanical_denudation.yaml:40
tests/LCM/ACE/MiniErosionClean/materials_mechanical_melt.yaml:72,108
tests/LCM/ACE/ThermoMechanicalCuboid/materials_mechanical.yaml:33
All five set ACE Bulk Porosity: 0.60, matching the thermal default. That unanimity is itself the argument that 0.60 is the intended value and 0.0 was never meant to be reachable.
Options
- Match the thermal default. Change
J2Erosion_Def.hpp:19 to default 0.60. One line, consistent with every existing deck and with ACEThermalParameters, and a deck that omits the key gets the value it would have gotten on the thermal side anyway. Leaves the two sides of a coupled run agreeing by construction.
- Require the key.
ALBANY_ASSERT that a porosity source is present when the model is J2 Erosion. Loudest and most honest, since there is no physically meaningful default porosity for arbitrary sediment, but it breaks any existing external deck that relies on the current silent behaviour.
- Treat 0 as a sentinel, as
Permafrost does. Skip the fits entirely and use the residual values directly when no porosity source is given, with a one-time warning. Preserves today's numerical behaviour exactly while making it visible.
- Leave it and document it. Least work, and defensible if the intent was always that decks specify porosity, but the next person to hit this loses a day.
My preference is 1, with 2 as a reasonable stricter alternative if you would rather no one ever runs J2 Erosion without stating a porosity. Either way Permafrost needs no change; its 0.0 is correct as it stands.
Whichever you pick, it is a one-line change plus a rerun of the ACE suite. Note that if you pick 1 or 3 the ACE golds do not move, since every current deck sets the key explicitly.
cc @ebayats
Found while reading how porosity enters
J2Erosioncompared to the newPermafrostcap model. No test or production deck in the tree is affected today, so this is a trap for the next deck rather than a live bug. Reporting rather than fixing, because the choice among the options below is Elyce's.The three defaults disagree
ACEThermalParameterssrc/LCM/evaluators/ACEThermalParameters_Def.hpp:506,550Permafrostsrc/LCM/models/Permafrost_Def.hpp:176J2Erosionsrc/LCM/models/J2Erosion_Def.hpp:19In
Permafrostthe 0.0 is a deliberate sentinel and is safe. Porosity there does exactly one thing, override the crushable volumeW, and the override is guarded:Omit the key and the
Frozen Parameters/Thawed Parametersvalues ofWsimply stand. Nothing silently changes.In
J2Erosionthere is no such guard. Porosity is not a parameter with a fallback, it is the second coordinate of three empirical surface fits, and 0.0 flows straight into them as a real porosity value:What porosity 0 does to the fits
All three fits are bilinear in (ice saturation
x, porosityy) and dominated by the cross termx*y. Sety = 0and the cross term vanishes, leaving only negative contributions:E_fit_maxis in fact negative for every ice saturation whenever porosity is below about 0.205, and increasing the ice saturation makes it more negative, not less.The clamps on lines 577-581 then catch it:
so the material does not blow up. It pins to
ACE Residual Elastic ModulusandACE Soil Yield Strengthat every point, at every step, at every ice saturation. That is the fully thawed state. A deck that forgets the key gets aJ2 Erosionmaterial with no ice strength at all, with no error, no warning, and a run that completes normally and looks plausible.The failure is quiet in the worst way:
ACE_Ice_Saturationis still computed and still written to the output, so the Exodus file shows ice present and the mechanics behaving as if there were none.Current exposure
Every deck that actually instantiates
Model Name: J2 Erosionsets a porosity key, so nothing in the tree is wrong right now:tests/LCM/ACE/MiniErosion/materials_mechanical_denudation.yaml:34tests/LCM/ACE/MiniErosionClean/materials_mechanical_denudation.yaml:40tests/LCM/ACE/MiniErosionClean/materials_mechanical_melt.yaml:72,108tests/LCM/ACE/ThermoMechanicalCuboid/materials_mechanical.yaml:33All five set
ACE Bulk Porosity: 0.60, matching the thermal default. That unanimity is itself the argument that 0.60 is the intended value and 0.0 was never meant to be reachable.Options
J2Erosion_Def.hpp:19to default 0.60. One line, consistent with every existing deck and withACEThermalParameters, and a deck that omits the key gets the value it would have gotten on the thermal side anyway. Leaves the two sides of a coupled run agreeing by construction.ALBANY_ASSERTthat a porosity source is present when the model isJ2 Erosion. Loudest and most honest, since there is no physically meaningful default porosity for arbitrary sediment, but it breaks any existing external deck that relies on the current silent behaviour.Permafrostdoes. Skip the fits entirely and use the residual values directly when no porosity source is given, with a one-time warning. Preserves today's numerical behaviour exactly while making it visible.My preference is 1, with 2 as a reasonable stricter alternative if you would rather no one ever runs
J2 Erosionwithout stating a porosity. Either wayPermafrostneeds no change; its 0.0 is correct as it stands.Whichever you pick, it is a one-line change plus a rerun of the ACE suite. Note that if you pick 1 or 3 the ACE golds do not move, since every current deck sets the key explicitly.
cc @ebayats