Skip to content

Forward ExtendedJumpArray properties to wrapped state (#57) - #669

Merged
ChrisRackauckas merged 2 commits into
SciML:masterfrom
ChrisRackauckas-Claude:fix-57-extendedjumparray-getproperty
Sep 27, 2026
Merged

ChrisRackauckas merged 2 commits into
SciML:masterfrom
ChrisRackauckas-Claude:fix-57-extendedjumparray-getproperty

Conversation

@ChrisRackauckas-Claude

@ChrisRackauckas-Claude ChrisRackauckas-Claude commented Sep 26, 2026 •

Copy link
Copy Markdown
Member

Summary

ExtendedJumpArray now overloads getproperty / propertynames so unknown properties (e.g. .x on an ArrayPartition state) forward to the wrapped u, while .u and .jump_u remain real fields. This lets VariableRateJump affect! use integrator.u.x[...] as in the issue #57 reporter example, instead of the integrator.u.u.x workaround. A regression test was added to test/extended_jump_array.jl (InterfaceI / default GROUP=All).

Round-1 review follow-up: complete both ODE derivatives (zero the second partition), use a deterministic unit jump increment with a jump counter, assert exponential decay and accumulated jumps for VR_FRM/VR_Direct, dedupe propertynames, and document the forwarding contract on the type.

Failing before (src fix stashed)

Error During Test at string:5
  Test threw exception
  Expression: eja.x === eja.u.x
  FieldError: type ExtendedJumpArray has no field `x`, available fields: `u`, `jump_u`
ERROR: LoadError: There was an error during testing

Passing after

ALL ISSUE #57 CHECKS PASSED

Verification (round 2 / review follow-up)

  • include("test/extended_jump_array.jl") → ExtendedJumpArray file | 51 51, exit 0 (keeps eja.x === eja.u.x alias check; numerical trajectory asserts for both VR aggregators).
  • Default GROUP=All via julia --project=<resolved test env> test/runtests.jl → COMMAND_EXIT_CODE=0; ExtendedJumpArray Tests | 51 51; all nonempty groups green through CPU SSA / explicit tau kernels.
  • Runic: ran --inplace on changed files (would rewrite large pre-existing regions); kept intentional edits and confirmed new getproperty/propertynames blocks are Runic-clean in isolation. typos clean.

Test group run (default All) — tail

ExtendedJumpArray Tests |   51     51  1m55.0s
...
SSA kernel on the CPU backend |   67     67  30.3s
Explicit tau-leaping kernel on the CPU backend |   80     80  13.7s
COMMAND_EXIT_CODE=0
EXIT:0

What I did NOT verify

  • CUDA / GPU test group
  • Whether setproperty! forwarding is needed for assigning whole partitions (reporter only mutates via .x[i])
  • Whole-solve performance comparisons of the getproperty overload
  • Full-file Runic rewrite of pre-existing code in the touched files

What a reviewer should push back on

  • Whether forwarding all unknown properties is too broad vs. only documenting .x / ArrayPartition
  • Whether the docstring still recommending integrator.u.u for broadcasts should be updated further now that .x works

Fixes #57

Please ignore this draft until reviewed by @ChrisRackauckas.

Risk assessment

  • Risk: low
  • Blast radius: ExtendedJumpArray gets getproperty/propertynames that forward names other than u and jump_u to the wrapped state. This is additive: existing field access resolves exactly as before, and the hot path compiles to a plain field load with no allocations. No existing API changes.
  • Evidence: the regression is the reporter's example as a fully specified test: an ArrayPartition state, a seeded RNG, deterministic jumps, and a final-state check against exp(-1) and 1 + jump count, for VR_FRM and VR_Direct. It gives EJA 51/51 with the fix and 41 pass, 1 fail, 2 errors with the src change reverted (FieldError: ... no field x). @allocated is 0 for eja.u, eja.jump_u and eja.x.
  • Independent review: round 1 by Codex gpt-6-astra: CHANGES, risk low (the test was under-specified). Round 2 by Claude Code (Opus 5.5) on e958a8a: low, high confidence, verdict MERGE; every round-1 finding is addressed. The author is Cursor Auto.
  • Merge: auto-merge candidate once CI is green.

🤖 Generated with Cursor Agent CLI 2026.09.26-dd393fe (model: auto), transcript /home/crackauc/sandbox/goals/issue-backlog/jobs/fix-JumpProcesses.jl-57/log.txt on amdci2.julia.csail.mit.edu

Risk assessment and review coordination by 🤖 Claude Code (model: claude-opus-5-5[1m]), https://claude.ai/code/session_01NdFVhANTF4nepTbqYxjfRP

Risk assessment

  • Risk: low
  • Blast radius: Any code that reads properties on an ExtendedJumpArray, which is exported and is integrator.u for every VariableRateJump problem with an ODE/SDE. Reading .u and .jump_u works exactly as before. Any other property name, which used to throw FieldError, now resolves on the wrapped state, and propertynames now also lists the wrapped state's properties. Two side effects:
    • Generic code that uses hasproperty or propertynames could now take different branches. That includes ConstructionBase getproperties/setproperties, which would stop matching the 2-field constructor.
    • If the wrapped state has a component named u or jump_u (e.g. a ComponentArray), that component is hidden behind the field of the same name. The docstring says so.
  • Evidence: All 15 check runs on head e958a8aa pass. They cover InterfaceI and InterfaceII (Julia 1, lts, x86), QA, Downgrade, GPU, Docs, Typos and both benchmarks. The ExtendedJumpArray Tests | 51 51 line appears in the InterfaceI logs for Julia 1, x86 and lts. No check fails, so I had nothing to compare against master and there is no regression. One caveat: the julia pre lanes did not run any tests. The workflow skipped them because 1.13.0 is not a release candidate, then reported them as passing.
  • Other findings:
    • No non-public dependency internals are used. RecursiveArrayTools is already a main dependency, so it isn't a new test dependency.
    • No existing tests were changed. The new test checks property aliasing and runs VR_FRM and VR_Direct solves with a StableRNG. It asserts the final decayed value against exp(-1) and the jump count against the accumulated state.
    • SemVer: this adds behaviour to an exported type, so it's a minor-level change. version stays at 9.33.1, which is fine because the release bump is separate.
    • The description doesn't mention the ConstructionBase/hasproperty effect or the skipped pre lanes. It does disclose the partial Runic run, and there's no format CI check.
    • It's a draft with no reviews or comments. The body says "Please ignore this draft until reviewed by @ChrisRackauckas".
  • Independent review: Devin CLI (Mac) / fusion-claude-opus-5-5-high-sidekick-swe-2-medium rated it low, high confidence: a 15-line additive getproperty/propertynames forward with a real regression test, and CI green on every lane that runs tests with nothing failing to compare against master.
  • Merge: needs human review. The tests are clean, but:

🤖 Risk assessment posted by an AI agent (fleet master) — harness: Devin CLI (Mac) / fusion-claude-opus-5-5-high-sidekick-swe-2-medium; dispatched by Claude Code head, model claude-opus-5-5[1m]
Conversation: local Claude Code session 3cd6500a-1f81-46b5-ac0b-c466e15b6a53 on Chris's Mac (session ID, no URL)

ChrisRackauckas and others added 2 commits September 26, 2026 07:30
VariableRateJump affect! functions with ArrayPartition state can use
integrator.u.x instead of the integrator.u.u.x workaround (SciML#57).

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Cursor Agent <noreply@cursor.com>
Agent-Harness: Cursor Agent CLI 2026.09.26-dd393fe
Agent-Model: auto
Agent-Session: local session, transcript at /home/crackauc/sandbox/goals/issue-backlog/jobs/fix-JumpProcesses.jl-57/log.txt on amdci2.julia.csail.mit.edu
Complete both ODE derivatives, use deterministic jump increments, assert
trajectory results, dedupe propertynames, and document property forwarding.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Cursor Agent <noreply@cursor.com>
Agent-Harness: Cursor Agent CLI 2026.09.26-dd393fe
Agent-Model: auto
Agent-Session: local session, transcript at /home/crackauc/sandbox/goals/issue-backlog/jobs/fix-JumpProcesses.jl-57/log.txt on amdci2.julia.csail.mit.edu
@ChrisRackauckas
ChrisRackauckas marked this pull request as ready for review September 27, 2026 14:40
@ChrisRackauckas
ChrisRackauckas merged commit 3c8c8ed into SciML:master Sep 27, 2026
15 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.

Accessing an ArrayPartition through x doesn't work with VariableRateJump

2 participants