Skip to content

Test symbolic stoichiometry through public API#1511

Merged
ChrisRackauckas merged 2 commits into
SciML:masterfrom
ChrisRackauckas-Claude:agent/fix-symbolic-stoichiometry-public-api
Jul 21, 2026
Merged

Test symbolic stoichiometry through public API#1511
ChrisRackauckas merged 2 commits into
SciML:masterfrom
ChrisRackauckas-Claude:agent/fix-symbolic-stoichiometry-public-api

Conversation

@ChrisRackauckas-Claude

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

Copy link
Copy Markdown
Member

Ignore this PR until it has been reviewed by @ChrisRackauckas.

Summary

Replace the symbolic-stoichiometry test's construction of private ModelingToolkitBase affect internals with the documented public compilation path:

compiled_jsys = ModelingToolkitBase.mtkcompile(jsys)
@test isempty(ModelingToolkitBase.equations(compiled_jsys))

The test still inspects the generated symbolic jump system, verifies that it contains one jump, and checks that its symbolic affect contains Pre(V(t)). This addresses the review request to retain assertions about the generated symbolic jump and its equations.

Root cause

The failure bisects to ModelingToolkit commit 136462c52d4c983e4ba832893928f270bc399b47 (refactor: do not rely on metadata when constructing AffectSystem), whose direct parent 0b01708ffb4c545b888f859bdcbc505350f723ef passes the test. That refactor made the private AffectSystem(...; parent_sys) keyword mandatory, so Catalyst's test errored while reconstructing private internals rather than testing Catalyst behavior.

ModelingToolkitBase.mtkcompile, jumps, and equations are exported by their declaring module; mtkcompile has its own docstring and a rendered API-docs entry.

Local verification

The exact downstream test command used Julia 1.12.6, current ModelingToolkit master 77941f873102904935c798fdb3dcd2bbfbfe8d3a, and coverage:

GROUP=Modeling julia +1.12 --project=<downstream-env> --startup-file=no \
  -e 'using Pkg; Pkg.test("Catalyst"; coverage=true)'
  • Clean Catalyst master: Symbolic Stoichiometry reached 20 passes, then errored with UndefKeywordError: keyword argument parent_sys not assigned.
  • This PR: Symbolic Stoichiometry passed 30/30.
  • The patched full group continued successfully through Reaction Balancing. It then stopped at the independent Conservation Laws baseline issue: 242 passed, 7 pre-existing @test_broken assertions unexpectedly passed, and 17 remained broken. That separate issue is tracked by draft PR Activate corrected conservation-law tests #1512; this PR does not alter those markers.
  • git diff --check passed.
  • Julia 1.10 and 1.12 parsing passed.
  • Runic passed on the changed executable let block. The whole-repository Runic check remains nonzero on pre-existing formatting outside this focused diff.

Please ignore this draft until it has been reviewed by @ChrisRackauckas.

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

@isaacsas isaacsas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please modify the test to check the generated symbolic jump system and its symbolic equations are correct.

Copy link
Copy Markdown
Member Author

Audit scratchpad for commit 3d943c635106d42426472cc6c3d30380f9898ab5:

  • Clean-master Julia 1.10 GROUP=Modeling reproduced UndefKeywordError: keyword argument parent_sys not assigned in the symbolic-stoichiometry test.
  • The failure came from the test constructing and inspecting non-public ModelingToolkitBase internals.
  • Replaced that inspection with a public JumpProblem / solve end-to-end regression.
  • The focused symbolic-stoichiometry suite passes locally: 30/30 with ModelingToolkitBase 1.52.0.
  • The full Modeling group then reaches the independent, pre-existing DSL Options parameter-discovery regression (308 pass, 9 fail, 1 broken), tracked by Catalyst Modeling test group fails on current master #1502/Fix symbolic option parameter discovery #1503 and ModelingToolkit #4714.
  • Runic passed for the changed block; repository-wide Runic has extensive pre-existing differences outside this focused PR.
  • The PR's prerelease Extensions runner loss is infrastructure, not a test assertion.

No private dependency API remains in the changed regression, and no failing test was hidden.

Copy link
Copy Markdown
Member Author

PR CI follow-up:

The focused change is confirmed by both Julia 1.12 and prerelease Modeling lanes: Symbolic Stoichiometry | 30/30 passes.

Those lanes then exposed a second latent clean-base bookkeeping failure in test/network_analysis/conservation_laws.jl: seven pre-existing @test_broken assertions now produce “Unexpected Pass” (lines 150, 152, 179, 183, 287, 290, and 325). The suite reports 242 pass, 7 error, 17 broken. This is independent of this PR’s changed test and is being assigned a separate clean-base/version investigation; I will not fold unrelated broken-test changes into this focused PR without verifying every supported Julia lane.

The LTS Modeling lane instead reaches the already tracked DSL Options regression (308 pass, 9 fail, 1 broken). QA and prerelease Extensions failures are self-hosted runner losses.

Copy link
Copy Markdown
Member Author

Scheduled CI recheck at 07:01 EDT: the workflow has finished. The focused symbolic-stoichiometry regression passes 30/30 on Julia 1.12 and prerelease. The remaining red checks are unchanged and outside this one-test patch: Julia 1.12/prerelease Modeling reaches the seven conservation-law unexpected passes under the newer ModelingToolkitBase semantics, LTS reaches the separately tracked DSL Options regression, and QA/prerelease Extensions lost their self-hosted runners. The docs build and all unaffected package groups pass.

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

Copy link
Copy Markdown
Member Author

Addressed the requested symbolic-system coverage in 47e09852.

The final diff now retains the original assertions that jump_model(rs) produces one symbolic jump and that its affect contains Pre(V(t)). It replaces only the private AffectSystem / unhack_system reconstruction with public ModelingToolkitBase.mtkcompile(jsys) followed by an assertion that the compiled system has no equations.

On Julia 1.12.6 with current ModelingToolkit master, the clean checkout errors at 20 pass + 1 error with the missing parent_sys keyword; this version passes Symbolic Stoichiometry 30/30. The full GROUP=Modeling run then proceeds through Reaction Balancing and stops at the separate Conservation Laws unexpected-pass baseline tracked in #1512.

Copy link
Copy Markdown
Member Author

Fresh local recheck on 2026-07-20, using Julia 1.12.6 and the repository's defined Modeling group:

GROUP=Modeling timeout 3600 julialauncher +1.12 --project=. --startup-file=no \
  -e 'using Pkg; Pkg.test(; coverage=true)'

Both runs resolved ModelingToolkitBase 1.54.1.

  • Clean master (ae55024a) passed Reaction Structure 99/99, ReactionSystem Structure 419 pass + 2 broken, and Higher Order Reactions 38/38, then reproduced UndefKeywordError: keyword argument parent_sys not assigned at symbolic_stoichiometry.jl:235 (Symbolic Stoichiometry | 20 pass, 1 error, exit 1).
  • This branch (47e09852) passed Symbolic Stoichiometry | 30/30 in the same resolved environment.
  • The full branch run continued through DSL Options | 317 pass, 1 broken; thus the older 308/9/1 LTS result is not present in today's Julia 1.12 resolution.
  • The first later failure was the independent stale-marker case tracked by Activate corrected conservation-law tests #1512: Conservation Laws | 242 pass, 7 unexpected-pass errors, 17 broken, at lines 150, 152, 179, 183, 287, 290, and 325.

The dependency boundary remains ModelingToolkit commit 136462c52d4c983e4ba832893928f270bc399b47; its parent is 0b01708ffb4c545b888f859bdcbc505350f723ef. That change made the non-public AffectSystem constructor require parent_sys. This PR avoids adding a dependency on that internal and instead tests through public mtkcompile/equations APIs.

The local branch is clean, matches origin/agent/fix-symbolic-stoichiometry-public-api, and is zero commits behind current upstream/master.

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member Author

Follow-up from the current CI failure in Catalyst #1516:

  • All three current Modeling lanes reproduce this same test failure with released ModelingToolkitBase 1.54.1: Julia 1.10.11, 1.12.6, and 1.13.0-rc1 each stop at Symbolic Stoichiometry | 20 pass, 1 error with the missing parent_sys keyword.
  • Untouched current Catalyst master (ae55024af9289d201b353d27859b29909a5e3bde) reproduces it locally with exact ModelingToolkitBase 1.54.1.
  • I also checked the registered release boundary directly on that clean Catalyst commit. ModelingToolkitBase 1.51.1 completes the original private reconstruction with equations_empty=true; ModelingToolkitBase 1.52.0 raises UndefKeywordError: keyword argument parent_sys not assigned. This agrees with the commit bisect: 136462c52d4c983e4ba832893928f270bc399b47 is included in the 1.52.0 release line but not 1.51.1.

This strengthens the existing conclusion: #1511 fixes a latent Catalyst test dependency on non-public ModelingToolkitBase internals through the documented public compilation path. The callback compat changes in #1516 merely caused CI to encounter the already-tracked clean-base failure.

@ChrisRackauckas
ChrisRackauckas marked this pull request as ready for review July 21, 2026 14:21
@ChrisRackauckas
ChrisRackauckas merged commit b1dd548 into SciML:master Jul 21, 2026
16 of 22 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.

3 participants