JIT: two constants of the same name compiled to one constants[] slot

[lmoresi]

Two constants that share a display name were compiled as one constants[] slot, so any solve holding two of them silently used a single value. Every ViscousFlowModel names its viscosity \eta, so this is not a niche case.

The symptom

A layered viscosity built from two ViscousFlowModels returned the uniform-viscosity answer:

L2 vs the exact layered Couette profile	2.819e-1
L2 vs plain linear shear	1.502e-10

The contrast was entirely absent, and nothing upstream looked wrong. unwrap(model.flux[0,0]) gave 2.0*M0*v_0,0 + 2000.0*M1*v_0,0, the level sets held 0..1, the constituent models were distinct objects, and constants_manifest held both values — (0, \eta = 1.0) and (1, \eta = 1000.0). The expression and the manifest were correct. Only the emitted C was wrong.

The cause

_JITConstant (utilities/_jitextension.py) is a plain sympy.Symbol subclass. It never adopted the disambiguation UWexpression uses (docs/developer/design/SYMBOL_DISAMBIGUATION_2025-12.md), and it had neither half of it: no _hashable_content discriminator, and it constructed through Symbol.__new__, which is cached by name.

a, b = _JITConstant(0, name='same'), _JITConstant(1, name='same')
a is b                    # True   <- the same object
a._ccodestr, b._ccodestr  # constants[1], constants[1]

The second placeholder was the first object, and assigning its _ccodestr overwrote the first one's. Every occurrence then rendered as the same slot, so a two-material blend assembled as (phi_0 + phi_1) * constants[k] — one uniform viscosity.

The fix

Follow the same mechanism UWexpression uses: construct via Symbol.__xnew__ to bypass the instance cache, and put the slot index in _hashable_content, so identity is the slot rather than the name. Also __slots__ and __getnewargs_ex__ to match.

Separately, _extract_constants sorted the manifest with a tie-break on str(expr), which for a UWexpression is its current value — so two same-named constants could swap slots when a parameter changed. It now tie-breaks on instance_number, which is creation order and identical on every rank running the same script.

Verification

The composed-model route and the equivalent single-model createMask blend now agree to 2.5e-12 on the same problem (both 8.010e-2 against the exact layered profile, which is the nodal level set's own error, not the solver's), and neither is linear shear.

Two regression tests in tests/test_0103_jit_rampable_constants.py, one at the _extract_constants level and one end-to-end. Both were confirmed to fail with the fix reverted and pass with it.

pytest -m "level_1 and tier_a": see the check run below.

Note for the reviewer

This commit also exists on feature/particle-demos (PR #715), where it was found. The content is identical, so the branches merge cleanly — except tests/test_0103_jit_rampable_constants.py, where the feature branch's end-to-end test uses the MaterialSwarm API introduced there. Take the feature branch's version of that file when the two meet.

Underworld development team with AI support from Claude Code

🤖 Generated with Claude Code

https://claude.ai/code/session_01MSGAFeA7qYXgkuw9ud8F2G

[Copilot]

🟡 Changes recommended

_extract_constants()’s new sort key can still raise a TypeError when same-named constants mix instance_number=None and integer instance numbers, so the ordering needs a small normalization fix before merge.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Fixes a silent correctness bug in the JIT constant plumbing where two distinct constants sharing the same display name could collapse to a single constants[] slot in generated C, producing incorrect physics (notably multi-material viscosity blends). The PR also stabilizes constant-slot assignment ordering and adds regression coverage to prevent reintroductions.

Changes:

• Updates _JITConstant to bypass SymPy’s name-based symbol cache (__xnew__) and to disambiguate identity via _hashable_content() using the constants-slot index.
• Adjusts _extract_constants() ordering so same-named constants don’t swap slots when values change.
• Adds two regression tests covering both placeholder extraction and an end-to-end two-material Stokes solve.

File summaries

File	Description
src/underworld3/utilities/_jitextension.py	Fixes _JITConstant identity and makes constant-slot assignment deterministic for same-named constants.
tests/test_0103_jit_rampable_constants.py	Adds regression tests for same-named constant placeholders and an end-to-end multi-material layered-viscosity solve.
src/underworld3/constitutive_models.py	Adds an inline note explaining the historical symptom and pointing to the JIT fix/regression test.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[lmoresi]

Adversarial review found that the previous revision was not safe to merge, and the cause was a decision I made while responding to the first round of review.

The bug has two independent halves:

	mechanism	symptom without it
identity	_hashable_content + Symbol.__xnew__	two same-named placeholders are one object → uniform-viscosity answer, silently
ordering	the slot index in the name	sort_key() ties → Add term order follows the hash seed → ranks emit different C → getext aborts

The original fix did ordering. I replaced it with the structural identity fix and removed the name change, since the design doc deprecates name-munging — but sort_key() is derived from the name, so _hashable_content does nothing for it. Measured with only the identity half: three distinct compiled modules from one model across four hash seeds (a6cb2e83, a6cb2e83, df7e63dd, 2f8aadf7), and mpirun -np 2 aborting on 4 of 6 launches. With both halves: one module across five seeds, 0 of 6 aborting.

Three regression tests, each verified to fail on the specific bug it guards — including one that isolates the ordering half while the identity test still passes, demonstrating the pre-existing tests could not have caught it.

Two things I removed rather than kept:

• An mpirun-based determinism test I wrote first, deleted because it caught the bug 0 times in 6 with the defect present and confirmed. Whether two ranks disagree depends on their seeds; a coin-flip assertion is not a regression test. The hermetic seed sweep replaces it.
• A first version of the slot-permutation test that was vacuous: it ramped 1000 → 1e-6, and "1.00000000000000e-6" sorts as a prefix of "1.00000000000000", so nothing moved. 1000 → 0.5 does permute.

Also dropped the instance_number None guard from the previous round (charter §5 forbids guarding unreachable states, and the comment justifying it misdescribed UWexpression.__init__), and made the docstring raw — it contained \e, an invalid escape scheduled to become a SyntaxError.

Full level_1 and tier_a: 1186 passed. One unrelated timing failure in test_0054_hang_report under heavy concurrent load; 8/8 three times in isolation.

Underworld development team with AI support from Claude Code