Repository navigation
Conversation
…astic response The exponential coefficients alpha = exp(-dt/tau) and phi were formed from float(viscosity)/float(modulus). A viscosity written as an expression of position (a damage band, a temperature-dependent law) raised there and the Maxwell time fell silently to None, which the coefficient update reads as the viscous limit: the model lost its elastic response wherever the viscosity was not a plain number (measured on the box fault model: a first-step end velocity of 0.007, the viscous value, against 0.061 with a constant). The coefficients are now formed symbolically when the Maxwell time is an expression, in the shared helper and at both constitutive-model callers; a constant viscosity takes the numeric path as before. Test: tests/test_1093 — a uniform-valued expression must match the constant it equals, and a weak band must not make the block viscous. Both shown failing before the fix. Underworld development team with AI support from Claude Code
…egulariser The symbolic path floored tau with a literal 1e-300; the Charter's regulariser is uw.maths.functions.vanishing. The comment records where phi = (1 - alpha)/x loses digits (x below ~1e-12) and that order 1 does not use phi. Underworld development team with AI support from Claude Code
…oured Review of the ported fix (four findings): - The symbolic coefficients took the step as a number, frozen into the compiled kernel: a change of step left the kernels on an earlier step's alpha (Stokes 1-8 %, Navier-Stokes 77 % on a step change). The step now enters as the dt_elastic atom, so a new value reaches the kernels as a constant. - The Maxwell time is formed from the parameters' own atoms, so the units system scales it: a dimensional modulus with a varying viscosity raised SympifyError (alpha now evaluates to exp(-0.125) in a units model). - One helper, _etd_maxwell_time, for all three callers; the experimental TransverseIsotropicVEPSplitFlowModel still had the silent viscous fallback. - The test moves to test_1053 (test_109* is outside every CI batch) and covers order 1 and 2, a changing step (fails when the step is frozen as a number), the transversely isotropic model, and a weak band against the uniform block and the band's own viscous limit; all at 1e-10 against the constant. All five fail on a build without the fix. Underworld development team with AI support from Claude Code
Second review round (no blockers): at order 2 phi = (1 - e^-x)/x lost its digits as x -> 0 and was 0/0 where the modulus vanishes. phi is now -expm1(-x)/x (printed as C expm1), and the symbolic Maxwell time floors the modulus with the declared regulariser, so mu -> 0 is the elastic limit as on the numeric path. Test docstring states intent; a redundant dt_elastic assignment goes. Underworld development team with AI support from Claude Code
|
Internal adversarial review — two rounds, against builds with and without the fix. Round 1 (on the first port):
Round 2: no blockers. Nit fixed: phi = 0/0 where mu = 0 and lost digits as x → 0 — now Attacks that failed: constant ↔ expression switches mid-run (identical to the constant run both ways); a mesh-variable viscosity changed between solves (exact viscous scaling); recompiles (none on a step change: 0.5–1.0 s vs 0.4–0.9 s per solve); TI-VEP order 1/2 (≤ 1.2e-13); Not changed here (predates the branch): an unreadable No open issue covers this bug. Generated by Claude Code |
With units, float(par.sym) is the magnitude in the parameter's own units: 1e21 Pa s / 3.2e7 Pa is a Maxwell time in seconds, against a step of 100 kyr, so alpha = 1 - 3e-12 and the model was purely elastic. The helpers now read the non-dimensional value the kernels see (dt/tau = 0.1 in the new test, for the isotropic and transversely isotropic models). Pre-existing: the base read the same magnitudes. Underworld development team with AI support from Claude Code
|
Post-review commit 189aa88: ETD coefficients from the scaled parameters Found while reviewing the stacked multi-material branch. When η, μ and dt carry units,
New test: Generated by Claude Code |
- The solver sets an explicit history order on a model before assigning it, so a composite whose constituent needs a higher-order history is refused and the solver keeps the model it had (it was left holding the refused one). - With #851's scaled Maxwell time merged in, each constituent's ETD coefficient comes from its own scaled tau when eta, mu and dt carry units (test; the units test from #851 reads alpha from the model, where this branch keeps it). - Stale comments about the history's exponential coefficients removed; the guide states how a composite and a single model take the solver's order. Underworld development team with AI support from Claude Code
The exponential (ETD) Maxwell integrator formed alpha = exp(-dt/tau) and phi from
float(viscosity)/float(modulus). A viscosity written as an expression of position or of a field (a damage band, a temperature-dependent law) has no float; the Maxwell time fell toNone, which the coefficient update reads as the viscous limit (alpha = phi = 0). The model silently lost its elastic response wherever the viscosity was not a plain number (measured on a box fault model: first-step end velocity 0.007, the viscous value, against 0.061).What changes:
_etd_maxwell_time(eta, mu), used by all three callers (ViscoElasticPlasticFlowModel,TransverseIsotropicVEPFlowModel,TransverseIsotropicVEPSplitFlowModel): a number when both parameters are numbers, otherwise the ratio of the parameters' own expression atoms, so the kernels evaluate it pointwise and the units system scales it.ddt._update_exp_values(..., dt_symbol=...): the symbolic coefficients take the step as thedt_elasticatom, so a new step reaches the kernels as a constant (no recompile, no stale coefficient); phi is-expm1(-x)/x; the Maxwell time and the modulus are floored withuw.maths.functions.vanishing, so a vanishing modulus is the elastic limit as on the numeric path.tests/test_1053_ve_etd_variable_maxwell_time.py(level_2, tier_b): a numerically uniform expression (1 + 1e-12 x) must reproduce the constant to 1e-10 at order 1 and 2, across a changing step, and for the transversely isotropic model; a weak band is more compliant than the uniform block and far from the viscous limit of the same banded model. All five fail on development; the step-change test also fails when the step is frozen into the kernel as a number. 5 passed serial and onmpirun -n 2.The fix first appeared on
feature/tidal-shell(9e97670); it is ported here so it does not wait on that branch, and extended after review.Internal adversarial review, two rounds (findings and evidence in the comment below). Left open, all predating this branch: a
dt_elasticthat cannot be read as a number still reads as the viscous limit; ETD's Maxwell time uses the rawshear_viscosity_0, not the yield-modified viscosity; ETD-2 under a step traction gives a velocity that alternates in sign.Checklist (UW3 Style Charter — docs/developer/UW3_STYLE_CHARTER.md)
level_*andtier_*markers (S8)maybe_/try_/do_) and no commented-out code (S3, S4).array/mesh.X.coords— nowith ....access(...), nomesh.data(S7)pixi.toml/pixi.lockor other dependency changes riding in an unrelated PRUnderworld development team with AI support from Claude Code