Skip to content

Test graph edge order without symbolic repr#1515

Draft
ChrisRackauckas-Claude wants to merge 1 commit into
SciML:masterfrom
ChrisRackauckas-Claude:agent/restore-passing-graph-edge-order-test
Draft

Test graph edge order without symbolic repr#1515
ChrisRackauckas-Claude wants to merge 1 commit into
SciML:masterfrom
ChrisRackauckas-Claude:agent/restore-passing-graph-edge-order-test

Conversation

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member

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

Summary

  • Assert the exact reaction-to-complex-edge permutation returned by ComplexGraphWrap.
  • Remove the brittle comparison against repr strings whose commutative factor order changes across Symbolics versions.
  • Keep the adjacent edgelist[rxorder] == Graphs.edges(img) assertion unchanged.

Root cause

Commit a815230d89c2b21c7f9322545043f922654fc6f1 changed the label-order assertion from @test to @test_broken. Under the downgrade workflow's outer-Catalyst load order, the equality already passes, so @test_broken raises an Unexpected Pass. Under the current standalone Extensions environment, Symbolics prints the same commutative product as B(t)*k5 rather than k5*B(t), so a literal string assertion cannot be valid in both supported environments.

edgelabels is deterministically derived from reactions(rn), while rxorder is the mapping under test. Asserting rxorder == [1, 4, 6, 2, 5, 3] preserves the complete ordering invariant without coupling the test to symbolic pretty-printing.

Local validation

  • Julia 1.10.11: GROUP=Extensions julia +1.10 --project=. -e 'using Pkg; Pkg.test()' — passed, including Graph visualization 25/25 and the full Extensions group.
  • Exact outer-Catalyst/inner-extension downgrade reproducer with DiffEqBase 7.2.0 and JumpProcesses 9.26.0 — Graph visualization 25/25.
  • Runic 1.7.0 check of the changed assertion — passed.
  • A full-tree Runic check was also run; current master has broad pre-existing formatting drift, so no unrelated formatter rewrite is included here.

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

Copy link
Copy Markdown
Member Author

Investigation scratchpad

  • Clean base: ae55024af9289d201b353d27859b29909a5e3bde.
  • The initial targeted GROUP=Extensions control did not reproduce the Unexpected Pass because it activates the extension environment before loading Catalyst.
  • Matching GROUP=All load order—load Catalyst from the outer downgrade environment, then activate test/extensions—reproduced the Unexpected Pass deterministically. The mapping was rxorder = [1, 4, 6, 2, 5, 3].
  • Boundary checks against the unchanged graph test:
    • ae55024: 24 passes + 1 Unexpected Pass.
    • 9d3448a6e614f2ddcd7b5c4866a7347266b2b6fd: 25/25 passes.
  • A formal git bisect run across the 13-commit range identified a815230d89c2b21c7f9322545043f922654fc6f1 as first bad. Its relevant change was only @test edgelabels[rxorder] == labels@test_broken ....
  • Current standalone Extensions dependencies expose why restoring the literal string assertion is insufficient: the equivalent symbolic product prints as B(t)*k5, while the downgrade graph prints k5*B(t).
  • The replacement assertion checks the exact permutation directly. This is equivalent to the intended ordering invariant because edgelabels was deterministically constructed from reactions(rn); the independent adjacent assertion edgelist[rxorder] == Graphs.edges(img) remains unchanged.

Final local validation on commit fb848eaf:

  • GROUP=Extensions julia +1.10 --project=. -e 'using Pkg; Pkg.test()': passed. Graph visualization 25/25; complete Extensions group passed.
  • Exact outer-Catalyst/inner-extension downgrade reproducer, with Catalyst loaded from this branch, DiffEqBase 7.2.0, and JumpProcesses 9.26.0: 25/25 passed.
  • Runic 1.7.0 check of the changed assertion: passed. Full-tree Runic was also executed and reports unrelated pre-existing formatting drift on master; this focused PR does not include that rewrite.

ChrisRackauckas-Claude commented Jul 17, 2026

Copy link
Copy Markdown
Member Author

CI watch update — final terminal state

  • All directly relevant Extensions checks are green:
    • Extensions Tests (lts) — passed in 26m50s
    • Extensions Tests (1) — passed in 44m27s
    • Extensions Tests (pre) — passed in 41m48s
  • The long build job also passed in 1h42m23s.
  • Tests (1, QA) did not report a Catalyst test failure. During JET analysis, after reaching 1044/1455 top-level definitions, the hosted runner emitted:
    • The runner has received a shutdown signal.
    • Julia then exited with signal 15: Terminated.
      The LTS and prerelease QA lanes both passed.
  • I attempted to re-run only the affected QA job (87818281731) so the known Modeling failures would not be restarted:
    1. GitHub CLI/REST: HTTP 403, Must have admin rights to Repository.
    2. Connected GitHub app: HTTP 403, Resource not accessible by integration.
  • Maintainer action is therefore required: use Re-run job on the Julia 1 QA job.
  • The only other red checks are the three Modeling lanes, all failing at the same unchanged symbolic_stoichiometry.jl:235 call with UndefKeywordError: parent_sys not assigned. That clean-master regression is handled separately by draft Test symbolic jump affects through public API #1509; it is outside this PR's GraphMakie-only diff.
  • Every other test lane passed. The branch is still synchronized with its remote and has zero commits behind current origin/master.

No #1515 code change is indicated by the remaining red checks.

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