Repository navigation
Conversation
test_1079's `_checkpoint_fields` helper asserted the field's saved vector is at `uw_checkpoint/<name>`. #756 moved the reload payload under /restart/petsc, where the same vector sits one group deeper as `.../dms/<name>/vecs/<name>/<name>`, so the assertion fails on every current file and development has been red since #708 merged. Nothing was broken. By the time that line runs, both the write and the resume phases have already returned 0 and the saved metadata has already compared equal -- the restart works. The test is marked `xfail(strict=True, raises=SymbolicDiskContinuationMismatch)`, so only that one exception counts as the expected failure and an AssertionError from the layout check surfaces as a genuine failure, which is why a cosmetic relocation reads as a broken restart. The helper now accepts either spelling, so a file written by either version stays readable to the test. This is the same assumption that regressed #640 in the reader (fixed in #847): the payload is found by structure, not by one hard-coded path. Proven on one build: with the single-spelling assertion restored the test fails at line 34; with this change it reaches its intended strict xfail. Checkpoint and snapshot suite: 122 passed, 1 xfailed. How this reached development: #708 added this test and its CI was green, but that run predates #756 by a day. #708 then merged twelve hours after #756 without a re-run, so the layout change and the assertion never met in CI until afterwards. Underworld development team with AI support from Claude Code Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E87Q7KrpapxeQiLD1RiNXv
lmoresi
added a commit
to gthyagi/underworld3
that referenced
this pull request
Oct 13, 2026
Two CI failures on this branch, both reproduced locally and bisected
against development.
test_0011 `test_replaying_a_rewound_step_reproduces_it`
------------------------------------------------------
`state` was defined TWICE on SNES_AdvectionDiffusion_Composed: the
existing AdvectionDiffusionStepState property, and a new AdvDiffusionState
one further down the class. Python keeps the last definition, so the first
became dead code -- and with it went `has_solution` and the
`SNES_Scalar.state.fset` delegation, neither of which AdvDiffusionState
carried.
`has_solution` decides whether the next solve starts from the current
iterate or from zero. After `model.rewind()` it stayed True instead of
returning to False, so the replayed step began from a different initial
guess and converged to a different iterate within snes_rtol (1e-8). The
fields came back 2.04e-10 apart and `np.array_equal` refused them.
Measured with the same probe in two worktrees, everything else equal --
T, psi_star, dt_history, theta, delta_t and the SNES iteration count all
identical:
has_solution before the replay replay bit-identical
development False yes
this branch True no (2.04e-10)
AdvDiffusionState now extends AdvectionDiffusionStepState rather than
SnapshottableState, so it keeps has_solution; there is one `state`
property, and its setter delegates to the base before applying the extra
fields. The probe now reports False and 0.0, matching development.
Ruled out first, each rebuilt and re-run: the eager `mesh.cell_size()` in
EulerianSUPG.__init__, the `delta_t` setter change, and the
`_register_state_bearer` call. Reverting the whole file to development
fixed it, which is what localised the cause to this file.
test_0018 `test_every_history_scheme_declares_a_verdict`
--------------------------------------------------------
EulerianSUPGPC derives from _DDtBase directly, so it inherited the base
refusal and read as undeclared. It now declares its own verdict, grounded
in what the corrector does: `_apply_pc_correction` advances the rate and
temperature by hand on PETSc vectors outside the owning solver's
residual, so the step is never materialised as an operator to transpose,
and under `pc_converged` `_converge_pc_residual` chooses the number of
corrections from a residual test, which is piecewise constant in the data.
A refusal is a verdict, not a gap; it should be revisited if the
corrections are ever expressed as a residual the solver assembles.
test_0011 and test_0018: 28 passed. test_00[0-4]* and test_107* under
xdist: 445 passed, 1 failed -- that one being test_1079, which fails on
development too and is fixed separately in underworldcode#848.
Underworld development team with AI support from Claude Code
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E87Q7KrpapxeQiLD1RiNXv
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.
Development has been red since #708 merged.
test_1079_diffusion_disk_restart.py::test_symbolic_diffusion_fresh_process_disk_restartfails on every current file:Cause
_checkpoint_fieldsasserts the field's saved vector is atuw_checkpoint/<name>. #756 moved the reload payload under/restart/petsc, where the same vector sits one group deeper as.../dms/<name>/vecs/<name>/<name>. I dumped the written file to confirm nothing was lost — the vector and its section are both present, just relocated.Nothing is actually broken. By the time that line runs, both the write and the resume phases have returned 0 and
_assert_saved_metadata_equalhas passed: the restart works. The test carriesxfail(strict=True, raises=SymbolicDiskContinuationMismatch), so only that one exception counts as the expected failure and anAssertionErrorfrom the layout check surfaces as a hard failure — which is how a cosmetic relocation reads as a broken restart.Fix
The helper accepts either spelling, so a file written by either version stays readable to the test. This is the same assumption that regressed #640 in the reader, fixed in #847: find the payload by structure, not by one hard-coded path. #756 has now broken two consumers this way.
Evidence
Proven on one build: with the single-spelling assertion restored the test fails at line 34; with this change it reaches its intended strict xfail. Checkpoint and snapshot suite (
test_0003,test_0005_xdmf*,test_0007,test_0008,test_0010,test_0012,test_0020,test_0864,test_1074,test_1079): 122 passed, 1 xfailed. Both style gates clean; all 354 test files reachable.How this reached development
#708 added this test and its CI was green — but that run predates #756 by a day. #708 then merged twelve hours after #756 landed, without a re-run, so the layout change and the assertion never met in CI until afterwards. #677 and #689 both fail on this too, so it is blocking them rather than anything of their own.
Underworld development team with AI support from Claude Code
🤖 Generated with Claude Code
https://claude.ai/code/session_01E87Q7KrpapxeQiLD1RiNXv