Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should periodic clock and event clock get same result? #3557

Closed
TongYuan-MC opened this issue Aug 9, 2024 · 11 comments · Fixed by #3567
Closed

Should periodic clock and event clock get same result? #3557

TongYuan-MC opened this issue Aug 9, 2024 · 11 comments · Fixed by #3567
Assignees

Comments

@TongYuan-MC
Copy link

model TestModel1
  Boolean b = sample(0, 0.1);
  Real x1 = sin(time);
  Real y1;
  Real y2;
  Real y3;
equation 
  when b then
    y1 = x1;
  end when;

  when Clock(0.1) then
    y2 = sample(y1);
  end when;

  when Clock(b, 0.1) then
    y3 = sample(y1);
  end when;
end TestModel1;

When simulated in OpenModelica 1.21.0, we see the following result:

image

y2 and y3 give different result. This behavior is incomprehensible to the users.

@henrikt-ma
Copy link
Collaborator

In System Modeler, the clock of y3 doesn't tick at all, but I guess that must be a bug?

@casella
Copy link
Collaborator

casella commented Aug 9, 2024

As I understand, there is no reason why y1 and y2 should give the same result. y1 is a discrete variable driven by the event-generating sample operator, while y2 is a clocked variable driven by a periodic clock.

The two concepts (discrete and clocked variables) are completely different and there is no guarantee that they are synchronous with each other just because you are using the same numerical value for the sample operator and for the Clock operator.

In fact, clocked variables were created exactly to avoid this kind of ambiguity, i.e., to avoid that synchroncity depends on numerical time values and dubious magic involving small epsilons, rather giving a formal definition of syncronous partitions, which are guaranteed to change at the same time in a consistent way.

That's why y1 and y3 are the same, they are both ultimately driven by the same clock with period 0.1. Note that if I define two different clocks with period 0.1, they are actually the same clock, so their clocked partitions are merged together, and they are guaranteed to be synchronous.

If you really worry about syncronicity, you should avoid using discrete variables and only use clocked variables instead.

BTW, OpenModelica's output should be improved here, as the clocked variables are only defined at the clock instants, contrary to discrete variables that are also defined outside their switching time points, see OpenModelica/OpenModelica#12770, but that's another story.

It is also true that this model may work in some other tools than OpenModelica, which employ better magic to handle events, or maybe are just luckier in this specific case. My point is, if you only use clocked variables you have formal guarantees of syncronicity. That's much better than relying on magic.

@HansOlsson
Copy link
Collaborator

That's why y1 and y3 are the same, they are both ultimately driven by the same clock with period 0.1. Note that if I define two different clocks with period 0.1, they are actually the same clock, so their clocked partitions are merged together, and they are guaranteed to be synchronous.

Well, even if they are ultimately given by the same clock there's a small difference.

Note that for y2 we can intuitively explain its value as follows - y2=sample(y1) is defined as the left-limit of the first argument. At 0.1 s the left-limit of y1 is 0 - and y2 becomes 0.

That doesn't happen for y3=sample(y1), where y1 has been updated before sampling.

I can see the reason for Dymola (which generates the same result as OpenModelica; except that we only plot the signals when they tick) and also that it was implemented more than a decade ago. It might be something we could include in the specification.

My point is, if you only use clocked variables you have formal guarantees of syncronicity. That's much better than relying on magic.

Agreed.

@casella
Copy link
Collaborator

casella commented Aug 9, 2024

Well, even if they are ultimately given by the same clock there's a small difference.

Can you please elaborate on that? I remember I had this discussion on some ticket (not sure if it was a Modelica or an OpenModelica ticket) and the conclusion was that clocked partions generated by the same literal Clock expression should be merged together. Am I mistaken here?

@HansOlsson
Copy link
Collaborator

Well, even if they are ultimately given by the same clock there's a small difference.

Can you please elaborate on that? I remember I had this discussion on some ticket (not sure if it was a Modelica or an OpenModelica ticket) and the conclusion was that clocked partions generated by the same literal Clock expression should be merged together. Am I mistaken here?

Yes.

  1. They are not in the same clock partition, as y1 is in the unclocked base-partition, and y3 in a clocked base-partition (with an event clock that happen to coincide with y1 updates). And y2 is in a different clock base partition (with a real clock.)
  2. The specification of base partitions in https://specification.modelica.org/master/synchronous-language-elements.html#modelica:clock-solver only allows multiple rational clocks in one base partition, so for other clocks the merging would lead to an error. But even rational clocks should still not be merged since:
  3. Merging them isn't good as solverMethod deduction would then spread "randomly".

Having the code generation merge partitions would be possible, but doesn't impact the semantics and thus doesn't matter from the language perspective.

@casella
Copy link
Collaborator

casella commented Aug 13, 2024

I apologize, I'm not sure what I had in mind when I wrote the the second part of my comment, starting from "That's why y1 and y3 are the same".

Of course y1 is not even in a clocked partition, so my comment didn't make much sense.

@HansOlsson are we on the same page if we state that given the current language specification, there is no reason for y1, y2, and y3 to change simultaneously?

@casella
Copy link
Collaborator

casella commented Aug 13, 2024

What I referred to about equal clocks being merged is demonstrated by the following MWE:

model SameClock1
  Real x1 = sin(time);
  Real y1;
  Real y2;
equation
  when Clock(0.1) then
    y1 = sample(x1);
  end when;
  when Clock(0.1) then
    y2 = sample(x1);
  end when;
end SameClock1;

The clocked variables y1 and y2 belong to two different clock partitions, although generated by identical clocks. As I read in MLS Section 16.7

[As clock partitions are solely determined by the equations, two different clock partitions can have clocks defined by the same expressions. It is a quality of implementation issue that such partitions are executed synchronously, e.g., by putting them in the same task in a real-time simulation context.]

the equations determining y1 and y2 may (but also may not) end up in the same run-time task, depending on quality of implementation. However, I understand that this model

model SameClock2
  Real x1 = sin(time);
  Real y1;
  Real y2;
  Real y3;
equation
  when Clock(0.1) then
    y1 = sample(x1);
  end when;
  when Clock(0.1) then
    y2 = sample(x1);
  end when;
  y3 = y1 - y2;
end SameClock2;

is not legal, because y1 and y2 belong to two different partitions. So, if the difference between y1 and y2 cannot be legally computed, this means there is no guarantee that the two variables will always be the same.

@HansOlsson is that correct?

@HansOlsson
Copy link
Collaborator

Yes, Francesco, that is consistent with my interpretation.

@HansOlsson
Copy link
Collaborator

BTW: The reason for the difference in Dymola (and assumedly something similar in OpenModelica) is that Event clocks don't use the event directly, but sort of use pre(event) (assumedly to avoid systems of equations in some cases).

To me it would make sense to update the specification to either:

  • Explicitly allow this
  • Make it mandatory

@HansOlsson HansOlsson added this to the 2024-September milestone Aug 30, 2024
@henrikt-ma
Copy link
Collaborator

BTW: The reason for the difference in Dymola (and assumedly something similar in OpenModelica) is that Event clocks don't use the event directly, but sort of use pre(event) (assumedly to avoid systems of equations in some cases).

Could you provide an example where such equation systems are avoided, so that I can compare with our handling?

@HansOlsson
Copy link
Collaborator

Poll for change:
1 No change
2 Explicitly allow pre for event clock. (Corresponding to when Clock(pre(b), 0.1).)
3 Make pre for event clock mandatory (Corresponding to when Clock(pre(b), 0.1).)

Henrik (1 or 3), Elena(abstain), Markus(abstain), Gerd(abstain), Dag(2 or 3).
Conclusion: 3 seems best, make PR.

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 a pull request may close this issue.

4 participants