Quickfix batch: clone() arguments (#498), print_table keywords (#499), and 56 orphaned test files rejoined to CI (#504) - #532
Merged
Conversation
…arsymbol (#498) The enhanced wrapper declared clone(self) and forwarded nothing, while the base MeshVariable.clone requires (name, varsymbol). Passing the arguments every in-tree caller uses raised TypeError, and the zero-argument form the wrapper offered would have failed inside the base — so clone() could not be called at all, and six shipped examples aborted on it. clone(name, varsymbol) now mirrors the base signature and returns an EnhancedMeshVariable (same mesh/shape/type/degree/continuity, independent data), so chained clones and enhanced features keep working. Regression test covers each documented argument, clone-of-clone, data independence, and discontinuous spaces; all four failed with the reported TypeError before the fix. Fixes #498 Underworld development team with AI support from Claude Code
…#499) The move to the PETSc-log backend cut print_table down to (filename, format), but 18 shipped examples still call it with the old display_fraction / group_by / output_file keywords and aborted with TypeError on the first timing report. output_file is restored as a working alias for filename (same behaviour: the table is written to that file; passing both is an error). The PETSc log backend cannot honour display_fraction (no row culling) or group_by (events, not lines/routines), so those are accepted with a visible FutureWarning rather than raising or being silently swallowed. Regression tests cover the exact example call patterns, that output_file really writes, the filename/output_file conflict, and that the current (filename, format) interface is unchanged; four of them failed with the reported TypeError before the fix. Fixes #499 Underworld development team with AI support from Claude Code
…n CI (#504) The serial batch globs skipped test_1018_rotated_freeslip.py entirely, so the 19-test rotated free-slip suite (including the #497 regression guard) never ran in CI. The one-off audit the issue asked for found 56 orphaned files in total. Batches now cover test_101*/test_102* (rotated free-slip, stokes MG, boundary flux), test_105* (VE/VEP, ddt, yield), test_005[1-9]*/test_006[0-1]* (mesh/deform/callback utilities), test_02* (solver smoke), test_1120*/ test_1450*, and the six named (un-numbered) test files - every added batch verified green locally with the exact test.sh pytest invocation. test_106* and test_107* stay out deliberately: they carry level_2/level_3 + slow + tier_b/tier_c suites and need a triage decision before gating CI (noted in the script). Also fixed in passing: the inline comment on the test_1110 line swallowed its "|| status=1", so annulus adv-diff failures could never fail the script. Fixes #504 Underworld development team with AI support from Claude Code
Contributor
There was a problem hiding this comment.
Pull request overview
This PR batches three small compatibility/CI fixes: it restores the intended EnhancedMeshVariable.clone(name, varsymbol) interface, reintroduces legacy uw.timing.print_table() keywords for example compatibility (with deprecation warnings), and updates scripts/test.sh so previously orphaned test modules are collected in CI.
Changes:
- Fix
EnhancedMeshVariable.clone()to accept and apply(name, varsymbol)and return an enhanced variable instance with independent data. - Extend
uw.timing.print_table()to accept legacy keywords (display_fraction,group_by,output_file) while preserving the current(filename, format)interface. - Expand
scripts/test.shbatching globs to include previously unrun serial tests (notablytest_1018*and other audited orphans), and fix an inline-comment bug that prevented failures from propagating.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/test_0508_enhanced_variable_clone.py | Adds regression coverage for the corrected enhanced-variable clone(name, varsymbol) behavior. |
| tests/test_0055_timing_print_table.py | Adds regression coverage for print_table() legacy keywords and conflict handling. |
| src/underworld3/timing.py | Restores legacy print_table() keyword acceptance via compatibility warnings and an output_file alias. |
| src/underworld3/discretisation/enhanced_variables.py | Implements the corrected EnhancedMeshVariable.clone(name, varsymbol) to return an enhanced clone. |
| scripts/test.sh | Rejoins orphaned serial test files to CI batches and fixes a comment-related ` |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+215
to
+219
| warnings.warn( | ||
| "print_table(output_file=...) is deprecated; use " | ||
| "print_table(filename=...) instead.", | ||
| DeprecationWarning, | ||
| stacklevel=2, |
This was referenced Aug 12, 2026
Closed
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.
Fixes #498, fixes #499, fixes #504
Three small independent fixes, one commit each.
#498 — EnhancedMeshVariable.clone() drops its arguments. The enhanced wrapper declared
clone(self)and forwarded nothing, while the baseMeshVariable.clonerequires(name, varsymbol)— so clone was uncallable in both directions and six shipped examples aborted on it.clone(name, varsymbol)now mirrors the base signature and returns anEnhancedMeshVariable(same mesh/shape/type/degree/continuity, independent data), so chained clones (v1 = v0.clone(...), as in Ex_Stokes_Cartesian_SolC) keep the enhanced type. New regression suitetests/test_0508_enhanced_variable_clone.py(4 tests) covers each documented argument, clone-of-clone, data independence and discontinuous spaces; all four failed with the reportedTypeError: clone() takes 1 positional argument but 3 were givenbefore the fix. Spot-run: Ex_Darcy_1D_benchmark.py now runs past its clone calls, both Darcy solves and rendering (it still trips much later on a pre-existing, unrelated matplotlib shape issue —evaluateoutput shaped(100,1,1)fed toplt.plot).#499 — timing.print_table dropped display_fraction/group_by/output_file. The PETSc-log rewrite cut the signature to
(filename, format); 18 examples still pass the legacy keywords and aborted withTypeErrorat their first timing report.output_fileis restored as a working alias forfilename(identical behaviour — the table is written to the file; passing both raisesTypeError). The PETSc backend cannot honourdisplay_fraction(no row culling) orgroup_by(events, not lines/routines), so those are accepted with a visibleFutureWarningrather than raising or being silently swallowed — per the issue's own analysis that silent ignoring of behaviour-changing keywords would be worse than raising, while a hard error keeps the 18 examples broken. New regression suitetests/test_0055_timing_print_table.py(6 tests) covers the exact example call patterns (including the Kramer three-keyword form), thatoutput_filereally writes, the conflict case, and that the current(filename, format)interface is unchanged; four failed with the reportedTypeErrorpre-fix.#504 — test_1018_ in no scripts/test.sh batch, plus the orphan audit.*
test_101*py tests/test_102*pynow runs as a serial batch, so the rotated free-slip suite (19 tests, incl. the #497 guard) is live in CI; verified locally with the exact test.sh invocation:pytest --config-file=tests/pytest.ini tests/test_1018*py→ 19 passed in 52 s.One-off audit of every
tests/test_*.pyagainst the batch globs found 56 orphaned files (not counting the deliberately disabledtest_0050*auditor andtest_06xxregression batches). Added to batches after verifying each batch passes locally:test_005[1-9]*+test_006[0-1]*(mesh/vtk/deform/callback utilities, 43 tests, 4 s)test_02*(solver smoke/warm-start/wallclock, 27 tests, 16 s)test_101*+test_102*(stokes DMPlex/nullspace/MG, rotated free-slip, boundary flux, FMG lockout, MG bundle — 91 tests, ~2.5 min total)test_105*(VE/VEP, ddt, yield smoother/homotopy/anchor, solve report — 78 tests + the existing test_1050, 5.3 min)test_1120*+test_1450*(vector SL, vector projection, 6 tests, 17 s)test_docstring_utils,test_jit_cache,test_jit_deterministic_ordering,test_multicomponent_projection,test_snes_vector_asymmetric_jacobian,test_symbol_disambiguation_prototype(74 passed, 1 skipped, 21 s)Also fixed in passing: line 77 read
$PYTEST tests/test_1110*py # Annulus version || status=1— the inline comment swallowed the|| status=1, so annulus adv-diff failures could never fail the script. Same gap class as CI: the rotated free-slip suite (test_1018_*) is in no scripts/test.sh batch — its regression tests never run #504.Left as listed findings (not added):
test_106*(Nitsche/constrained free-slip, jacobian layout, Newton cold start) andtest_107*(free surface plume / moving mesh / spherical): these carrylevel_2/level_3+slow+tier_b/tier_cmarkers (test_1064 is level_3 + slow + tier_c), and a probe run of the 106x batch was still grinding after 20 min wall / 12 min CPU — not a "trivially safe" add. They need a triage/deselect decision before gating CI; noted with a comment in test.sh.tests/parallel/has its own orphans: the parallel batch runs onlytests/parallel/test_075*py, sotest_0005_xdmf_viz_topology_mpi,test_0700_*,test_076x–test_079x,test_0855_*,test_1017_*, andtest_106x_*_parallelthere never run via test.sh. Verifying those needs mpirun time and belongs in its own pass.Gate:
pixi run -e amr-dev pytest tests -m "level_1 and tier_a" -q -p no:cacheprovider --ignore=tests/test_0050_utils.py→ 590 passed, 0 failed, 17 skipped (need--with-mpi), 1 xfailed, in 7:21.Underworld development team with AI support from Claude Code