Skip to content

Conversation

@henrikt-ma
Copy link
Collaborator

We decided to make both delay and spatialDistribution event triggering operators, but so far the specification has only been updated for delay. This PR does it also for spatialDistribution.

At least to me, trying to capture the intricacies of event triggering in the pseudo-code for spatialDistribution doesn't sound like a good idea. Instead, I'd like us to consider the option of throwing out the pseudo-code, now that it is becoming even more remote from an actual implementation.

Note that we haven't test-implemented this in System Modeler, so I am counting on others to verify that the few things I have added regarding event triggering is in agreement with what has already been "test-implemented" in other tools.

@HansOlsson
Copy link
Collaborator

Note that this will take some time, due to the need to check the implementations (which are more complicated than for delay).

@henrikt-ma
Copy link
Collaborator Author

I had the impression that we already had the necessary test implementations in some tools? @casella, @gkurzbach?

@HansOlsson
Copy link
Collaborator

I had the impression that we already had the necessary test implementations in some tools? @casella, @gkurzbach?

Well, at least it will take more time for me to check (including checking whether we have a test-implementation).

Specifically for the test-implementations it would be good to understand exactly which events are propagated (just discrete-time expressions? any change? any minimum discontinuity to avoid events bouncing back and forth and generating more and more events?)

@henrikt-ma
Copy link
Collaborator Author

henrikt-ma commented Dec 16, 2025

Specifically for the test-implementations it would be good to understand exactly which events are propagated (just discrete-time expressions?

Just like delay, I thought it was obvious that we were not restricting this to discrete-time expressions.

However, I realize that spatialDistribution has a potential source of discontinuity which we don't have for delay, namely the change of transport direction. However, I would hope that it is the model's choice of event generation for positiveVelocity that will determine whether a change of direction should result in a discontinuity in the internal distribution. Would that make sense?

any change? any minimum discontinuity to avoid events bouncing back and forth and generating more and more events?)

I'd say we should specify this with the same level of detail as for delay. This is how it is formulated for delay, and I will add a similar statement to this PR:

It is a quality of implementation to avoid excessive generation of events by only preserving significant discontinuities.

@henrikt-ma henrikt-ma force-pushed the spatialdistribution-events branch from 80a8fab to 68d5382 Compare December 16, 2025 22:19
@HansOlsson
Copy link
Collaborator

I notice that it was now generalized to non-Reals. I can to some extent see that it makes sense, but I'm not sure if I have seen any use-case for it - and I don't know how much extra issues it brings. It may be easier to skip that part.

However, I realize that spatialDistribution has a potential source of discontinuity which we don't have for delay, namely the change of transport direction. However, I would hope that it is the model's choice of event generation for positiveVelocity that will determine whether a change of direction should result in a discontinuity in the internal distribution. Would that make sense?

I don't know the consequence of this. I think I get the point that non-events for positiveVelocity imply that the spatialDistribution doesn't generate events and thus isn't discrete-time, even if you send in non-Reals; but there might be additional subtle issues.

@henrikt-ma
Copy link
Collaborator Author

I notice that it was now generalized to non-Reals. I can to some extent see that it makes sense, but I'm not sure if I have seen any use-case for it - and I don't know how much extra issues it brings. It may be easier to skip that part.

Yes, I found it best to put everything on the table. Based on my experience with implementing delay, it shouldn't be too difficult to support non-Real types once the discrete-time variability is supported properly.

@henrikt-ma
Copy link
Collaborator Author

I don't know the consequence of this. I think I get the point that non-events for positiveVelocity imply that the spatialDistribution doesn't generate events and thus isn't discrete-time, even if you send in non-Reals; but there might be additional subtle issues.

At least I have tried to separate the concerns of event generation within spatialDistribution itself from event-generation used externally for the determination of positiveVelocity. The key was to include the variability of positiveVelocity (not anything about event-generation per se) in the conditions for the spatialDistribution expression being discrete-time.

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