Skip to content

Conversation

@ChrisRackauckas-Claude
Copy link

Summary

This PR fixes issue #3907 where systems with identical continuous events were no longer considered equal after the v10 update.

The root causes were:

  1. zero_crossing_id was generated via gensym() for each callback, making otherwise identical callbacks unequal
  2. AffectSystem equality used isequal for its internal System, but System uses reference equality (===)

Changes:

  • Exclude zero_crossing_id from AbstractCallback equality and hash (it's an internal identifier, not semantically meaningful for equality)
  • Use isapprox instead of isequal for system comparison in AffectSystem equality (since System has a proper isapprox implementation for structural comparison)
  • Hash AffectSystem based on semantic content (equations, observed) rather than system identity

Before this fix:

using ModelingToolkit
using ModelingToolkit: t_nounits as t, D_nounits as D
@parameters p d
@variables X(t)
eqs = [D(X) ~ p - d*X]
continuous_events = [X ~ 1] => [X ~ Pre(X) + 1.0]
@mtkcompile sys1 = System(eqs, t; continuous_events)
@mtkcompile sys2 = System(eqs, t; continuous_events)

# Events were not equal even though they're semantically identical
e1 = ModelingToolkit.continuous_events(sys1)
e2 = ModelingToolkit.continuous_events(sys2)
isequal(e1[1], e2[1])  # false

After this fix:

isequal(e1[1], e2[1])  # true
hash(e1[1]) == hash(e2[1])  # true

# Systems with same name are now approximately equal
sys3 = System(eqs, t; continuous_events, name=:test)
sys4 = System(eqs, t; continuous_events, name=:test)
sys3 = mtkcompile(sys3)
sys4 = mtkcompile(sys4)
isapprox(sys3, sys4)  # true

Test plan

Fixes #3907

🤖 Generated with Claude Code

This PR fixes issue SciML#3907 where systems with identical continuous events
were no longer considered equal after the v10 update.

The root causes were:
1. `zero_crossing_id` was generated via `gensym()` for each callback,
   making otherwise identical callbacks unequal
2. `AffectSystem` equality used `isequal` for its internal `System`,
   but `System` uses reference equality

Changes:
- Exclude `zero_crossing_id` from `AbstractCallback` equality and hash
  (it's an internal identifier, not semantically meaningful)
- Use `isapprox` instead of `isequal` for system comparison in
  `AffectSystem` equality
- Hash `AffectSystem` based on semantic content (equations, observed)
  rather than system identity

Fixes SciML#3907

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <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.

Identical systems that contain events are no longer equal

2 participants