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 clock partitions with consistent rational clocks be merged? #3265

Closed
casella opened this issue Oct 22, 2022 · 11 comments · Fixed by #3283
Closed

Should clock partitions with consistent rational clocks be merged? #3265

casella opened this issue Oct 22, 2022 · 11 comments · Fixed by #3283
Assignees
Labels
clarification Specification of feature is unclear, but not incorrect clocked Clocked parts of Modelica; synchronous and state-machines worksforme

Comments

@casella
Copy link
Collaborator

casella commented Oct 22, 2022

Consider the following MWE:

model SameClock
  Clock Clock1 = Clock(1,1000);
  Clock Clock2 = Clock(1,1000);
  Real x1(start = 0, fixed = true);
  Real x2(start = 0, fixed = true);
equation
  when Clock1 then
    x1 = previous(x1) + 1;
  end when;
  when Clock2 then
    x2 = previous(x2) + 1;
 end when;
end SameClock;

According to Section 16.3, which states

one of the following mutually exclusive associations of clocks are possible in one base partition:

  1. One or more rational interval clocks, provided they are consistent with each other, see section 16.7.5.

the variables x1 and x2 may belong to the same base clock partition (in fact to the same clock partition in this case, since the two clocks are identical).

The question is: are they required to be in the same clock partition?

The documentation of Modelica.Clocked.ClockSignals.Clocks.PeriodicExactClock states:

All clocks with rational number definitions are exactly time synchronized to each other

This seems to me to imply that x1 and x2 should belong to the same clock partition, i.e., that merging the base clock partitions that are consistent with each other is mandatory. However, I could not find any statement in the specification backing this requirement. BTW, the current OMC implementation doesn't do that.

Is that a quality of implementation issue? I don't think it should. I would find it a bit weird that end users employ PeriodicExactClock components in their models assuming that consistent clocks will be synchronized (because they read the block documentation, not the MLS), to later find out to their dismay they aren't.

@casella
Copy link
Collaborator Author

casella commented Oct 22, 2022

Adding @AnHeuermann, @kabdelhak, and @phannebohm in the loop.

@casella casella added clocked Clocked parts of Modelica; synchronous and state-machines clarification Specification of feature is unclear, but not incorrect labels Oct 22, 2022
@HansOlsson
Copy link
Collaborator

HansOlsson commented Oct 24, 2022

BTW: Following the specification it should be Clock Clock1=Clock(1,1000);

The question is: are they required to be in the same clock partition?

No, and conceptually they aren't even allowed to be in the same clock partition in that model.
The idea is that you perform clock partitioning as specified, and then check the clocks to see that they satisfy that requirement.

The goal is instead to consider the following models:

model Sub
   input Clock clock;
   Real x;
 equation
   when clock then
     x=previous(x)+1;
  end when;
end Sub;

model SameClock1
  Sub sub1(clock=Clock(1,10));
  Sub sub2(clock=Clock(1,10));
  Real y=sub1.x+sub2.x;
end SameClock1;

model SameClock2
  Sub sub1(clock=Clock(1e-1));
  Sub sub2(clock=Clock(1e-1));
  Real y=sub1.x+sub2.x;
end SameClock2;

model SameClock3
  Sub sub1(clock=Clock(1,10));
  Sub sub2(clock=Clock(2,10));
  Real y=subSample(sub1.x)+sub2.x;
end SameClock3;

Here SameClock1 and SameClock3 are legal, but SameClock2 is not legal (it would be straightforward to allow SameClock2 but as soon as there are subSample etc it would be a mess).

Without the Real y=... line sub1.x and sub2.x would be part of two different clocked partitions, that just happened to tick simultaneously.

The documentation of Modelica.Clocked.ClockSignals.Clocks.PeriodicExactClock states:

All clocks with rational number definitions are exactly time synchronized to each other

This seems to me to imply that x1 and x2 should belong to the same clock partition, i.e., that merging the base clock partitions that are consistent with each other is mandatory. However, I could not find any statement in the specification backing this requirement. BTW, the current OMC implementation doesn't do that.

Clocks can be synchronized (in the sense of ticking at the same time) without being part of the same clock partition. (But if they are in the same clock partition they must be synchronized.)

@casella
Copy link
Collaborator Author

casella commented Oct 25, 2022

BTW: Following the specification it should be Clock Clock1=Clock(1,1000);

Sorry, I mixed up several versions of the MWE and eventually copied the wrong one. Corrected.

@casella
Copy link
Collaborator Author

casella commented Oct 25, 2022

OK. So

  • clock partitions are entirely determined by the incidence of equations
  • one can have multiple clock partitions with exactly the same clock (or the same base clock) but that doesn't mean they belong to the same clock partition
  • it is a quality of implementation that clocked variables ticking exactly at the same time but not related by equations are actually synchronized

In particular, if I added

Real e = x1 - x2;

to the SameClock model, then it would have only one clock partition, while if I just plot x1 - x2 in the GUI, the fact that is shows no glitches is a quality-of-implementation issue.

Also, if the synchronous Modelica code was used to generate real-time code, it is a quality of implementation issue that the variables in different (but synchronized) partitions are put in tasks that are actually executed at the same time, or possibly even merged in the same task, eventually.

@HansOlsson do I get everything right?

@HansOlsson
Copy link
Collaborator

@HansOlsson do I get everything right?

I think so, yes.

@HansOlsson HansOlsson self-assigned this Nov 20, 2022
@HansOlsson
Copy link
Collaborator

Will see if can be clarified.

@casella
Copy link
Collaborator Author

casella commented Nov 21, 2022

The non-normative addition helps clarifying the issue. I wonder if we could add to 0bbfdf something like "It is a quality of implementation that such partitioned are executed synchronously, e.g. by putting them in the same task in a real-time simulation context.". Or is it too much?

@HansOlsson
Copy link
Collaborator

The non-normative addition helps clarifying the issue. I wonder if we could add to 0bbfdf something like "It is a quality of implementation that such partitioned are executed synchronously, e.g. by putting them in the same task in a real-time simulation context.". Or is it too much?

Added (after slight changes).

@casella
Copy link
Collaborator Author

casella commented Nov 21, 2022

Good! Where do I see that?

@HansOlsson
Copy link
Collaborator

@casella
Copy link
Collaborator Author

casella commented Nov 21, 2022

LGTM. Thanks!

HansOlsson added a commit that referenced this issue Feb 9, 2023
* Clarify that clock is only determined from equations.
Closes #3265
Co-authored-by: Henrik Tidefelt <henrikt@wolfram.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clarification Specification of feature is unclear, but not incorrect clocked Clocked parts of Modelica; synchronous and state-machines worksforme
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants