Skip to content

Fix underdetermined thermal heat-flow sensor test#488

Merged
ChrisRackauckas merged 1 commit into
SciML:mainfrom
ChrisRackauckas-Claude:fix/thermal-heat-flow-sensor-topology
Jul 13, 2026
Merged

Fix underdetermined thermal heat-flow sensor test#488
ChrisRackauckas merged 1 commit into
SciML:mainfrom
ChrisRackauckas-Claude:fix/thermal-heat-flow-sensor-topology

Conversation

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member

This PR should be ignored until reviewed by @ChrisRackauckas.

Summary

  • Connect the two ideal HeatFlowSensors in series instead of in parallel.
  • Keep both sensors in the test and assert that they report exactly the same through-flow.

Why

The parallel topology connects both ideal sensors across the same two thermal nodes. Their zero-temperature-drop equations are duplicates, while the node balances constrain only the sum of the two sensor flows. The individual flow split is therefore arbitrary, producing the observed structural imbalance (11 highest-order variables and 10 equations on the current stack).

Putting the sensors in series gives each sensor a uniquely determined through-flow without changing production code. This also preserves the original heat-flow assertion and adds a direct regression assertion for both sensor readings.

The invalid parallel topology dates to the initial thermal test upload (ae289f4d). A local release boundary probe with the unchanged fixture passes on ModelingToolkit v10.31.0 and is detected as structurally singular on v10.31.1. The detecting source change is ModelingToolkit commit 3eef54d43a752f7a46a0f53689d5f557115299f9, which moves consistency checking before alias elimination so alias-added zero constraints no longer mask underdetermination.

The output-marking approach explored in #450 does not change structural balance; #452 also identified the parallel sensors.

Verification

  • Julia 1.10.11, exact current dependency graph: full test/thermal.jl passes (Heat systems 6/6, Heat flow system 6/6, Radiator 3/3, Thermal Collector 3/3, FixedHeatFlow 2/2).
  • Julia 1.12.6, exact current dependency graph: full test/thermal.jl passes with the same counts.
  • Julia 1.13.0-rc1, exact current dependency graph: full test/thermal.jl passes with the same counts.
  • Runic --check . passes on the full repository.
  • git diff --check passes.

A clean-main GROUP=Core run was also attempted locally. It reaches the pre-existing piston missing-independent-variable failure before the thermal file; the targeted thermal suite above was therefore run directly on all three Julia versions.

Connect the two ideal heat-flow sensors in series so the test system has a unique through-flow, and assert that both sensors report the same value.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member Author

Investigation scratchpad

Reproduction

I reproduced this from unmodified upstream main at 2a9fc95eaf2c8934ffc35c0bf13d84921578e768 using Julia 1.12.6 and the current resolved stack (ModelingToolkit 11.31.2, ModelingToolkitBase 1.51.1, StateSelection 1.11.0, SciMLBase 3.34.0, OrdinaryDiffEq 7.1.2, SciCompDSL 1.0.1, SymbolicUtils 4.40.0, Symbolics 7.31.2).

Compiling the unchanged heat-flow system fails with:

ExtraVariablesSystemException: 11 highest order derivative variables and 10 equations
candidates: mass1₊T(t), hf_sensor2₊Q_flow₊u(t)

Structural cause

Each ideal heat-flow sensor imposes zero temperature drop and has an independent through-flow variable. With both sensors connected across the same two nodes, their temperature equations are duplicates and the node balances determine only q1 + q2. The difference q1 - q2 is unconstrained. Marking a sensor value as an output cannot add the missing equation, which is why the approach in #450 still failed; #452 separately called out the parallel sensor layout.

git log --follow -S hf_sensor2 -- test/Thermal/thermal.jl test/thermal.jl traces this parallel topology to the initial test upload, ae289f4d885c153d75078e9b0c64eb2d9d7840d7.

Detection boundary

Using the unchanged fixture from MTSL v2.25.0 in one preserved environment:

  • ModelingToolkit v10.31.0: compilation succeeds (compiled with 10.31.0).
  • ModelingToolkit v10.31.1: compilation fails with InvalidSystemException, identifying hf_sensor2₊Q_flow₊u(t).

The source boundary is ModelingToolkit commit 3eef54d43a752f7a46a0f53689d5f557115299f9 (fix: avoid false positives in consistency check), whose parent is exactly the v10.31.0 tag. It moves check_consistency before alias elimination because alias elimination adds var ~ 0 constraints for underdetermined variables and had masked the invalid fixture.

Fix and verification

The patch connects both sensors in series and asserts exact equality of their readings. No production API or implementation changes are needed.

  • Julia 1.10.11: all thermal testsets pass; Heat flow system 6/6.
  • Julia 1.12.6: all thermal testsets pass; Heat flow system 6/6.
  • Julia 1.13.0-rc1: all thermal testsets pass; Heat flow system 6/6.
  • Full-repository Runic 1.7 check passes.
  • git diff --check passes.

The unmodified clean-main GROUP=Core run was attempted as well. All preceding Core files passed until the separately tracked piston test failed because its system lacks an independent variable; Core therefore never reached the thermal file in that run.

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member Author

Independent corroboration from the current stacked-branch audit:

  • The unchanged parallel-sensor fixture compiles with ModelingToolkit v11.9.1 (PARALLEL_SENSOR_COMPILE_PASS unknowns=1). It fails with v11.11.1, v11.15.0, v11.28.2, and current master, reporting 28 highest-order variables versus 27 equations and identifying hf_sensor2 as an extra variable.
  • A source-boundary check with the same resolved dependency graph passes at b0f31536949370d189c03df71bc38f692a9a6975^ and fails at exact commit b0f31536949370d189c03df71bc38f692a9a6975 (fix: avoid false positives in consistency check). This is a later v11-line detection boundary that corroborates the structural diagnosis documented here.
  • Before discovering this PR, I tested the weaker alternative of removing the unused second sensor. The full focused thermal file passed 19/19 on the current local MTK/MTKBase stack: Heat systems 6/6, Heat flow 5/5, Radiator 3/3, Thermal Collector 3/3, and FixedHeatFlow 2/2. The series topology in this PR is preferable because it fixes the same underdetermination while retaining and directly testing both sensors.

A broader Core run reached the thermal file and passed that focused repair, then stopped later at the separate clean-main AccelerationSensor structural-singularity failure; that untouched translational regression is being audited independently.

Copy link
Copy Markdown
Member Author

CI classification as the matrix progresses:

  • Core on Julia LTS/current/prerelease and the downgrade job all stop earlier in the clean-main piston fixture with the same _check_if_dde(..., PleaseImportDynamicQuantities, ...) MethodError. Draft Define the piston test independent variable #485 is the focused prerequisite. Because piston.jl precedes thermal.jl, none of these jobs reaches this PR's series-sensor regression.
  • QA, documentation, Runic, Runic suggestions, discovery, and spelling pass.
  • The ModelingToolkit downstream job is still pending, so this is not a terminal-run claim.

The changed thermal file has already passed locally in full on Julia 1.10, 1.12, and 1.13-rc1 (20/20 each), with no skipped or weakened assertion.

@ChrisRackauckas
ChrisRackauckas marked this pull request as ready for review July 13, 2026 20:00
@ChrisRackauckas
ChrisRackauckas merged commit e2b4642 into SciML:main Jul 13, 2026
6 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants