Conversation
Member
Author
|
Adversarial review before the push, one pass over the diff with probes run in the worktree. Found and fixed in the pushed commit:
Recorded, not fixed: the strain-rate forcing stores keep |
…ork arrays, one dimensional read (#788) Every history store is a non-dimensional work array behind the units boundary and carries no units of its own. In a model with reference quantities the histories failed in four ways: the nodal commit and the SUPG carry copied through unit-aware .array views (np.copy raised); the particle flavours wrote the dimensional values evaluate returns straight into storage and ran away (1e148 after 20 steps); the integration-point trace cache keyed on an unhashable unit-carrying timestep; and nothing read back in pascals. - The timestep is reduced where every flavour receives it. - Anything written from evaluate is reduced on the way in. - Copies between stores go through .data with the component layout (EnhancedMeshVariable gains the layout delegate). - DFDt.carried(level) is the one way to read a history back: the constitutive model supplies the map from the stored value to the stress, the value times the model's unit of stress, so the read is in pascals when reference scales are set and unchanged when they are not. Test: the Maxwell shear box with reference quantities, all five stress_transport flavours, read through carried and held to the loading curve in Pa at the order-1 tolerance. Underworld development team with AI support from Claude Code Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017kSkAq7oJ5J3XisuvLBovo
lmoresi
force-pushed
the
bugfix/stress-history-units
branch
from
September 26, 2026 01:10
55e12bf to
4462b7d
Compare
…eparate dimensional read (#788) Adversarial review of the previous commit found the separate read (carried, decode_history, a reference-stress expression, a private map handed from the solver) reimplements what MeshVariable units already do, and found defects the test could not see. Now: - A solver builds its stress history with the stress's units (the flux it hands over at construction is a zero placeholder and cannot say so). .data stays non-dimensional; .array and evaluate read in Pa. One helper, _history_units, replaces the derivation pasted into four flavours, and Eulerian/EulerianSUPG stores take units too. - One rule for writing an evaluation into a store (_write_evaluated): a result that carries units is reduced, one that does not is already non-dimensional; the re-wraps that assumed an unlabelled result was dimensional are gone, and so are the units attached before reducing in the inflow, integration-point and forward writes. The Eulerian and Lagrangian_Swarm initialisations were writing dimensional values unreduced. - The BDF-2 step-ratio guard compared a dimensional dt_elastic with the non-dimensional history and silently fell back to BDF-1 whenever the time unit differed from the reference; both are compared non-dimensionally. - The mirror write in the component copies (wrong for a full tensor, a no-op for a symmetric one) is removed; one timestep reducer, built on _as_float. - Swarm advection wrote the dimensional velocity global_evaluate returns into its model-unit arithmetic, so in a units model the particles did not move (every swarm-carried quantity, not only stress); the reads are reduced. Test: every flavour, in a units model with the timestep in kyr (not the reference unit) and as the same problem in plain numbers, from a moving start, orders 1 and 2: the stores agree to 1e-6 and the stress reads back in Pa as the non-dimensional value times the stress scale. The previous test was blind to the timestep, the order-2 guard, the initial writes and the swarm motion. Underworld development team with AI support from Claude Code Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017kSkAq7oJ5J3XisuvLBovo
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #785. Makes the viscoelastic stress histories, and swarm advection, work in a model with reference quantities.
Design. A history's stores hold non-dimensional values in
.data, which is what the solver reads and what copies between stores go through. A solver builds its stress history with the stress's units, so.arrayandevaluateof the store read in Pa, as for any MeshVariable. What enters is reduced on the way in: the timestep where each flavour receives it, and anything written fromevaluate(one rule,_write_evaluated: a result that carries units is reduced, one that does not is already non-dimensional).What failed before, Maxwell shear box with reference quantities (1 km, 1e21 Pa s, 1 Myr):
Test
test_1064_stress_history_units: every flavour, in a units model with the timestep in kyr (not the reference unit) and as the same problem in plain numbers, from a moving start, at orders 1 and 2. The stores agree to 1e-6, and the stress reads back in Pa as the non-dimensional value times the stress scale.The stress-history, swarm and advection test files pass (324, plus one known fragile test that now passes); the style gate is clean.
History of this PR. The first version stamped the flux's units onto every store; the second made the stores unitless with a separate dimensional read (
carried). Adversarial review found the second reimplemented MeshVariable units and that its test was blind to the timestep, the order-2 guard, the initial writes and the swarm motion; this version replaces both.Fixes #788.
Underworld development team with AI support from Claude Code
🤖 Generated with Claude Code
https://claude.ai/code/session_017kSkAq7oJ5J3XisuvLBovo