Uh oh!
There was an error while loading. Please reload this page.
fix(function): rbf derivative path uses the requested component, not component 0 (BF-01) - #312
Conversation
…component 0 (BF-01) The quick/Clement derivative-evaluation path (_clement_to_work_variable, reached via uw.function.evaluate(expr, pts, rbf=True)) contained a dead ternary, `comp = 0 if source_var.num_components == 1 else 0`, which evaluates to 0 on both branches. Every derivative expression was therefore served the component-0 gradient: v[1].diff(x) with rbf=True silently returned d(v[0])/dx. Per-component Clement gradients were already computed and stored keyed (source_var, component), and each derivative expression carries its own flat data-column index (diffcls.component, set at UnderworldFunction registration); only the retrieval discarded it. The retrieval loop now keys the gradient lookup by deriv_expr.component, which is correct for scalar, vector, and tensor variables alike. Reproducing the bug with a P2 vector variable exposed two adjacent defects on the same path, fixed here as well. First, the higher-degree nodal sampling step indexed var.sym[comp, 0] with the flat data-column index, but sym is a (1, cdim) row matrix for vectors, so any multi-component P2+ variable on the mesh raised IndexError before the ternary was reached; the sampling now evaluates the component's own applied function directly. Second, _evaluate_field_at_vertices assumed a component-blocked DOF layout for P2+ multi-component fields, but the PETSc Plex local vector is point-major with components interleaved per point, so the extracted vertex values (and hence the Clement gradients) were garbage for every component; the extraction now de-interleaves before taking the vertex block, verified exact against analytic fields. The accurate L2 projection path (rbf=False) was unaffected throughout, and no solver numerics are touched. Regression test tests/test_0507_rbf_derivative_component.py (level_1, tier_a) evaluates each component derivative of v = (x, 2y) on P2 and P1 vector variables, a scalar control, and a mixed-component divergence expression; all ten tests fail on the previous code and pass with the fix. Audit reference: docs/reviews/2026-07/REMEDIATION-WORKLIST.md (BF-01), LOOSE-ENDS-AUDIT.md (LE-01). Underworld development team with AI support from Claude Code
There was a problem hiding this comment.
Pull request overview
Fixes a long-standing correctness issue in the rbf=True (Clement / “quick”) derivative-evaluation path where derivative expressions for multi-component variables could incorrectly use component 0’s gradient (and, for P2+ vector fields, could error or produce garbage due to incorrect DOF/component handling). The changes align derivative-gradient retrieval with each derivative expression’s registered flat component index and correct vertex sampling for multi-component higher-degree fields.
Changes:
- Use each derivative expression’s own
component(flat data-column index) when retrieving per-component Clement gradients in_clement_to_work_variable. - Fix higher-degree nodal sampling by evaluating the component’s applied function directly instead of mis-indexing
var.sym. - Correct
_evaluate_field_at_verticesfor multi-component P2+ fields by de-interleaving the PETSc local vector and improving fallback component selection; add a regression test suite covering P1/P2 vector derivatives and mixed-component expressions.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/underworld3/function/_function.pyx | Fixes derivative-gradient lookup to use deriv_expr.component and avoids incorrect var.sym indexing for higher-degree nodal sampling. |
src/underworld3/function/gradient_evaluation.py | Corrects P2+ multi-component vertex extraction to match Plex point-major interleaved layout and improves fallback component selection. |
tests/test_0507_rbf_derivative_component.py | Adds regression coverage for per-component derivatives (P1 + P2), scalar control, and mixed-component expressions under rbf=True. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Uh oh!
There was an error while loading. Please reload this page.
…into quality-audit branch Underworld development team with AI support from Claude Code
Summary
Fixes BF-01 (finding LE-01) from the 2026-07 quality audit
(
docs/reviews/2026-07/REMEDIATION-WORKLIST.md,docs/reviews/2026-07/LOOSE-ENDS-AUDIT.md).The quick/Clement derivative-evaluation path
(
_clement_to_work_variableinsrc/underworld3/function/_function.pyx,reached via
uw.function.evaluate(expr, pts, rbf=True)) had a dead ternary,which evaluates to 0 on both branches, so every derivative expression was
served the component-0 gradient:
v[1].diff(x)withrbf=Truesilentlyreturned
d(v[0])/dx. Per-component Clement gradients were already computedand stored correctly, keyed
(source_var, component), and every derivativeexpression carries its own flat data-column index
(
diffcls.component = data_loc, set atUnderworldFunctionregistration) —only the retrieval discarded it. The accurate L2 path (
rbf=False) wasunaffected throughout.
Reproduction (pristine source)
v = (x, 2y)on a P1 vector variable, interior points,rbf=True:Reproducing with a P2 vector variable (the typical velocity case) exposed
two adjacent defects on the same evaluation chain, fixed here as well:
var.sym[comp, 0]withthe flat data-column index, but
symis a(1, cdim)row matrix forvectors, so any multi-component P2+ variable on the mesh raised
IndexError: Index out of range: a[1]before the ternary was reached._evaluate_field_at_vertices(gradient_evaluation.py) assumed acomponent-blocked DOF layout for P2+ multi-component fields, but the
PETSc Plex local vector is point-major with components interleaved per
point — the extracted vertex values, and hence the Clement gradients,
were garbage for every component of a P2 vector (verified: sample error
2.0 on the unit box; de-interleaved extraction is exact, error 0.0).
Fix
component:
gradient_at_nodes[(source_var, deriv_expr.component)].Correct for scalar (data_loc 0), vector (data_loc = component), and
tensor (data_loc = flat
_data_layoutindex) variables, matching thekeys under which the gradients are stored.
function (
varfn) directly instead of mis-indexingvar.sym._evaluate_field_at_verticesde-interleaves the local vector beforetaking the vertex block for multi-component P2+ fields, and its
evaluate-fallback selects the applied function by matching
componentrather than positional
symindexing.No solver numerics are touched; the changes are confined to the rbf/Clement
evaluation path.
Tests
tests/test_0507_rbf_derivative_component.py(level_1, tier_a),10 tests: P2 vector per-component derivatives (4 parametrized cases),
P1 vector per-component derivatives isolating the ternary bug (4 cases),
scalar-variable control, and a mixed-component divergence expression.
All fail on the previous code (P2 cases by IndexError, P1 cases by wrong
component) and pass with the fix:
10 passed in 3.29s..pyxrebuilt from a cleanbuild/(stale-C guard) and the behaviourchange verified against the installed extension before trusting results.
pytest -m "level_1 and tier_a" -x -q(run in threefile-range batches), PRE fix on pristine build:
tests/test_0[0-4]*: 76 passedtests/test_0[5-9]*(excl. new test): 151 passed, 1 skipped, 4 xfailed, 1 xpassedtest_1*etc. +tests/parallel): 68 passed, 6 skipped(151 + 10 new), 1 skipped, 4 xfailed, 1 xpassed / 68 passed, 6 skipped.
Identical outcomes to PRE apart from the 10 new passing tests.
(no swarm/migration/partition-sensitive code).
Underworld development team with AI support from Claude Code