Conversation
When multiple simultaneous events fire (e.g. DiscreteHazard basis events), the code tries to 'bring matches up to speed' via pull_back(l, m) ⋅ r. If a prior simultaneous event invalidated the match, pull_back returns nothing and the composition crashes with MethodError. Fix: check pull_back result for nothing and skip the invalidated event (continue to next), similar to the existing dangling condition check. (cherry picked from commit d01abc7)
kris-brown
left a comment
kris-brown
left a comment
There was a problem hiding this comment.
Thanks! This is very helpful to avoid a cryptic error message.
Is it possible to create a small test corresponding to this change? It would create two conflicting simultaneous events and then catch an error with a nice error message. If you want the functionality of skipping invalidated events rather than error, a test could also show how to specify this as a simulation parameter.
| m = pull_back(l, m) ⋅ r | ||
| pb = pull_back(l, m) | ||
| if isnothing(pb) | ||
| @debug "Skipping event $(name(rule)): match invalidated by prior simultaneous event" |
There was a problem hiding this comment.
I think that by default there should be an error (with a helpful error message!) if there are conflicting simultaneous events, though I could also imagine a keyword flag to do this behavior (of giving the simultaneous events an arbitrary order and ignoring invalidated ones).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Updated this PR to match the requested behavior. It now raises a clear error by default when one simultaneous event invalidates another, and adds a regression test that constructs exactly that conflict with two simultaneous loop-removal rules on the same graph. |
2 participants
Problem
When multiple simultaneous events are processed in one timestep, the runtime tries to update each pending match so it reflects rewrites that already happened earlier in the same batch.
That update step does
pull_back(l, m) ⋅ r. If one simultaneous event invalidates another events match,pull_back(l, m)returnsnothing. Before this PR, that led to a cryptic composition failure instead of a clear explanation that the batch of simultaneous events was internally inconsistent.Fix
This PR now treats that situation as a genuine conflict in the simultaneous-event set and raises a clear error by default.
When a later simultaneous event can no longer be updated because a prior rewrite invalidated its match,
run!now throws anErrorExceptionexplaining that conflicting simultaneous events were encountered and identifying the invalidated rule.This keeps the failure mode explicit and reviewable instead of silently skipping work or crashing with a lower-level
MethodError.Testing
DiscreteHazard(1.)loop-removal rules on the same one-loop graph"Conflicting simultaneous events"error message