Skip to content

Fix rotational mechanics initialization and torque balance#486

Merged
ChrisRackauckas merged 2 commits into
SciML:mainfrom
ChrisRackauckas-Claude:agent/fix-rotational-core-regressions
Jul 13, 2026
Merged

Fix rotational mechanics initialization and torque balance#486
ChrisRackauckas merged 2 commits into
SciML:mainfrom
ChrisRackauckas-Claude:agent/fix-rotational-core-regressions

Conversation

@ChrisRackauckas-Claude

@ChrisRackauckas-Claude ChrisRackauckas-Claude commented Jul 13, 2026

Copy link
Copy Markdown
Member

This PR should be ignored until reviewed by @ChrisRackauckas.

Depends on #485. This branch is stacked on that piston-test fix so CI can exercise the rotational changes on a complete green baseline. Once #485 merges, this PR will reduce to the rotational commit.

Summary

  • Initialize both driven inertias at zero angular velocity instead of constraining an acceleration in the ODE test.
  • Restore the two-flange torque balance that was lost when TorqueSensor moved from @mtkmodel to @component.
  • Check the measured torque against the fixed support reaction, which is its exact physical counterpart in this test topology.

The existing atol = 1 conservation check is unchanged.

Root cause

With the acceleration constraint, the preloaded spring and damper force the second inertia to start at 500π rad/s. On current ModelingToolkit this produced:

w1_initial = 0.0
w2_initial = 1570.7963267948967
maximum_residual = 3141.910963169249
count_residual_gt_1 = 101

Explicitly starting both inertias from rest gives a maximum conservation residual of 0.31813746890592665, below the unchanged tolerance.

The missing TorqueSensor flow balance separately left the sensor model with 23 highest-order variables and 22 equations. After restoring it, the sensor torque is exactly opposite the fixed support reaction (maximum(abs.(sensor_tau .+ fixed_tau)) == 0.0).

Source-history bisects identified:

  • 27fa668cc920d1de71d724978b926e43758e3526 (test(Rotational): fix offsets and sol definitions) as the first commit with the current driven-inertia initialization/offset pattern.
  • 5d56791e142853b1eeeb34eccdaa7a4edfc430dc (refactor: use @component syntax instead of @mtkmodel) as the first commit where TorqueSensor no longer receives the connector flow balance generated by @mtkmodel.

Verification

Against ModelingToolkit 34ee9182900688f8545d61fb92d143ed29a8ad7f on Julia 1.12.6, in separate processes:

Piston cylinder wall                 |  3 pass /  3 total
two inertias                         |  6 pass /  6 total
two inertias with driving torque     |  6 pass /  6 total
first example                        |  1 pass /  1 total
Stick-Slip                           |  2 pass /  2 total
sensors                              | 12 pass / 12 total
Position                             |  4 pass /  4 total

Also passed:

Runic 1.7.0 --check: exit 0
git diff --check: exit 0

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

Copy link
Copy Markdown
Member Author

Investigation scratchpad:

  1. Reproduced both failures on clean MTSL 933a983a with MTK aa4afff870: the driven-inertia tolerance assertion and the 23-variable/22-equation sensor model.
  2. Reduced the driven model and measured the unintended 500π rad/s initial velocity; verified explicit zero velocities pass the existing tolerance without loosening it.
  3. Bisected the initialization pattern to 27fa668c and the lost TorqueSensor flow balance to 5d56791e.
  4. Restored torque conservation and verified the measured torque is exactly the negative fixed-support reaction.
  5. Ran the complete rotational test file (all 31 assertions passed), repository-wide Runic 1.7.0, and git diff --check locally.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
@ChrisRackauckas-Claude
ChrisRackauckas-Claude force-pushed the agent/fix-rotational-core-regressions branch from 73e0d2e to 93ebf68 Compare July 13, 2026 16:22
@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member Author

Stacked this branch on prerequisite #485 and force-updated it with an explicit lease after verifying the remote tip was unchanged.

Fresh local verification against ModelingToolkit 34ee918290 on Julia 1.12.6:

  • test/piston.jl: 3/3 passed in 2m03.3s.
  • test/rotational.jl: 31/31 passed across all six testsets (6 + 6 + 1 + 2 + 12 + 4).
  • Runic 1.7.0 repository check: exit 0.
  • git diff --check: exit 0.

The earlier #486 failures were the piston error fixed by #485; this stack lets CI reach the rotational change.

Copy link
Copy Markdown
Member Author

Independent verification against the published stacked tip 93ebf68d552c62e7476c6d69d9a719edd4f83a92 completed on Julia 1.12.6 with ModelingToolkit 11.31.2. The exact targeted command

timeout 3600 julia +1.12 --startup-file=no --project=mtksl-rotational-env test/rotational.jl

exited 0 with 31/31 assertions passing (two inertias 6/6, driving torque 6/6, first 1/1, stick-slip 2/2, sensors 12/12, position 4/4). The test retains the existing atol = 1; this PR does not loosen tolerances.

A clean-main native GROUP=Core Pkg.test() exited earlier at the independently known piston import failure, before reaching rotational.jl; that prerequisite is handled by #485. Historical inspection independently confirms two distinct rotational boundaries: 27fa668c introduced the inconsistent initialization pattern and 5d56791e dropped the TorqueSensor balance equation.

Copy link
Copy Markdown
Member Author

Fresh LTS CI reaches and passes the intended rotational coverage: Core/rotational.jl is 31/31, and the stacked piston prerequisite is 3/3. It also shows the earlier downgrade-only FixedHeatFlow case passing 2/2 on this graph.

The job later fails in an unrelated thermal test before package completion: Heat flow system errors during mtkcompile at test/thermal.jl:93 with ExtraVariablesSystemException (11 highest-order derivative variables, 10 equations; candidates mass1.T and hf_sensor2.Q_flow.u). The #486 production diff only changes Rotational/Sensors; its test diff changes only piston/rotational fixtures, so this is outside the PR patch. Per the clean-main audit rule, I am queuing a separate exact-main reproduce/bisect rather than changing #486 or weakening the test.

Copy link
Copy Markdown
Member Author

The refreshed downgrade job is the same clean-main thermal failure already isolated from #485: Core/thermal.jl reports 18 pass/1 fail only because SciMLBase.successful_retcode(sol) is false in the FixedHeatFlow with alpha=0.0 case. Piston is 3/3 and rotational is 31/31 on this exact downgrade graph. A separate clean-main reproduce/bisect is already running; no #486 changes are warranted for this failure.

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member Author

The current Julia 1.12.6 and pre-release Julia 1.13.0-rc1 lanes now confirm the same clean-main thermal structural error as LTS: piston passes 3/3 and rotational passes 31/31, then test/thermal.jl:64-93 errors while compiling Heat flow system with 11 highest-order derivative variables versus 10 equations (candidate extras mass1.T and hf_sensor2.Q_flow.u). FixedHeatFlow passes 2/2 in both of these lanes. This is one shared clean-main audit, separate from the downgrade-only FixedHeatFlow retcode investigation; neither warrants altering #486.

@ChrisRackauckas
ChrisRackauckas marked this pull request as ready for review July 13, 2026 20:06
@ChrisRackauckas
ChrisRackauckas merged commit 0646438 into SciML:main Jul 13, 2026
6 of 11 checks passed
ChrisRackauckas added a commit that referenced this pull request Jul 21, 2026
Releases the rotational mechanics initialization/torque balance fix from #486, currently unreleased on master.



Claude-Session: https://claude.ai/code/session_01SFmcAmLrGPrGtzwP333mX8

Co-authored-by: ChrisRackauckas-Claude <accounts@chrisrackauckas.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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