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

Can noClock be used between different base-clocks? #2365

Closed
HansOlsson opened this issue May 16, 2019 · 31 comments · Fixed by #2670
Closed

Can noClock be used between different base-clocks? #2365

HansOlsson opened this issue May 16, 2019 · 31 comments · Fixed by #2670
Labels
decided A decision has been made (label added before the spec is changed)
Milestone

Comments

@HansOlsson
Copy link
Collaborator

Following #1094 and #2182 I noticed one issue:

The noClock-operator is listed as a sub-clock conversion operator, but it is also implied in that issue that it can convert between different base-clocks.

That should be clarified, and possibly noClock moved to another section. The description should say that noClock may be used to convert a value from one clock to another (that may or may not be part of the same base-clock partition). That also implies that noClock does not connect different partitions for solverMethod, right?

@miscco
Copy link
Collaborator

miscco commented May 16, 2019

As far as I understand it that should be fine. Basically noClock(x) is equivalent to previous(x) but is ignored by Clock Partitioning

@HansOlsson
Copy link
Collaborator Author

As far as I understand it that should be fine. Basically noClock(x) is equivalent to previous(x) but is ignored by Clock Partitioning

Ok. And I assume this means that noClock(u) is also legal in the continuous-time partition (but still u must be clocked)?

@miscco
Copy link
Collaborator

miscco commented May 16, 2019

No, it is explicitely stated, that noClock returns a a variable and that the Clock is inferred. So you cannot use it in the continuous-time partition.

As I understand it, the whole purpose of noClock is precisely the interaction between different Base-Clocks. Note that there is no real way for different base Clocks to interact and within the same baseClock noClock is equal to previous.

In fact the phrasing makes even more sense in the context of different Base-Clocks, as there is no guaranteed ordering between Clocks. Consequently the only value one can get from an other Base-Clock is that of the last tick.

I think we should improve the wording to explicitely highlight that use case and explain the reasoning better.

I will try to come up with a better wording tomorrow.

miscco added a commit that referenced this issue May 17, 2019
We had two issues (#2365 and #2355) with noClock so try to improve the wording to be more explicit about the use case and the expected return value
@carlj-w
Copy link

carlj-w commented May 17, 2019

I disagree. noClock cannot be used between different base-clocks, it is meant to convert between sub partitions where you don't care what the relation between the sub partition is (if it is subSample, superSample, etc), for convenience. This is why it is under section 16.5.2, "Sub-clock conversion operators".

Also note that under section 16.7.3, "Base-clock Partitioning ", noClock is not one of the exceptions in incidence(e), but it is exempt in 16.7.4, "Sub-clock Partitioning".

I also don't think noClock is the same as previous.

For previous we have: "[...] the value of u from the previous clock activation is returned."

For noClock we have "At every tick of the clock of y, the operator returns the value of u from the last tick of the clock of u.", [here I presume last means latest, i.e. it is not the value from the final clock tick, as that would not make sense].

The latest clock tick might be the (sub)clock tick happening now, not the previous one.

@HansOlsson
Copy link
Collaborator Author

I disagree. noClock cannot be used between different base-clocks, it is meant to convert between sub partitions where you don't care what the relation between the sub partition is (if it is subSample, superSample, etc), for convenience. This is why it is under section 16.5.2, "Sub-clock conversion operators".

The reason for this ticket was questioning whether that made sense, and obviously if we decide that noClock can be used to connect different base-clock partitions updates would be needed. Thus I don't think we can dismiss it that easily.

Note in #1094 (specifically #1094 (comment) ) it was stated that it could connect different base-clock partitions without anyone noticing anything odd.

On the other hand connecting different base-clock partitions is inherently dangerous from a synchronous perspective; whereas different sub-clock partitions can more safely be mixed (I think), so using the same primitive to "cast" in both cases seems problematic.

@miscco
Copy link
Collaborator

miscco commented May 17, 2019

it is meant to convert between sub partitions where you don't care what the relation between the sub partition is

That would be terrible in that the model would be completely non-deterministic. If you indeed take the value of the "current" clock tick then all hell breaks loose with different Implementation leading to different Simulation results, as they can order the Subpartitions in some random way.

I think that sample(hold(u)) without EventIteration is the most reasonable use case. On the other hand subSample(previous(u)) is a bit of a mouth full.

@carlj-w
Copy link

carlj-w commented May 17, 2019

The reason for this ticket was questioning whether that made sense, and obviously if we decide that noClock can be used to connect different base-clock partitions updates would be needed. Thus I don't think we can dismiss it that easily.

And points have been made that this is already allowed, which I think it is not.

Note in #1094 (specifically #1094 (comment) ) it was stated that it could connect different base-clock partitions without anyone noticing anything odd.

Which doesn't mean there isn't something odd. According to the rules for base clock partitioning your partition would have had two different clocks, both Real clocks, which would not be allowed.

On the other hand connecting different base-clock partitions is inherently dangerous from a synchronous perspective; whereas different sub-clock partitions can more safely be mixed (I think), so using the same primitive to "cast" in both cases seems problematic.

I agree with this.

@carlj-w
Copy link

carlj-w commented May 17, 2019

That would be terrible in that the model would be completely non-deterministic. If you indeed take the value of the "current" clock tick then all hell breaks loose with different Implementation leading to different Simulation results, as they can order the Subpartitions in some random way.

No, when sorting your equations you have to take the whole base partition into consideration.

@carlj-w
Copy link

carlj-w commented May 17, 2019

That would be terrible in that the model would be completely non-deterministic. If you indeed take the value of the "current" clock tick then all hell breaks loose with different Implementation leading to different Simulation results, as they can order the Subpartitions in some random way.

No, when sorting your equations you have to take the whole base partition into consideration.

Note that this is not unique to noClock, the same thing goes for subSample, superSample, etc., you have to compute the input to these operators before (or "at the same time") you use the equations they are in.

@carlj-w
Copy link

carlj-w commented May 17, 2019

In summary, my position is the following:

noClock cannot be used between base clock partitions. To allow it, we must introduce either:

  1. Some kind of ordering between base clock partitions.
  2. Some kind of implicit delay when using noClock between base partitions.

I think (1) is complicated (would probably need a whole MCP). I think (2) is confusing (because noClock would behave differently between base clocks versus between sub clocks, or the behavior of noClock between subclocks needs to be changed). Also, in case of (2) you can also just use sample(hold(...)).

In the specification we can clarify the purpose of noClock, that it is an interface between subclock partitions without needing to specifying the sampling relation.

@miscco
Copy link
Collaborator

miscco commented May 17, 2019

I fully agree that noClock is a nice convenience for the user. However, what I do not see is the benefit from the simulation perspective.

In contrast there is a clear benefit for noClock when dealing with different Basepartitions.

So for me the question is, whether we should open noClock up to allow for different Basepartitions or introduce a different operator because there is considerable value in that functionality with respect to what sample(hold(u)) offers.

@HansOlsson
Copy link
Collaborator Author

I'm unsure if (2) would work even with an implicit delay, since the two base-clocks may not only tick at the same "time" - but also at slightly different times close together (that was after all one of the issues that synchronous extension would solve).

@carlj-w
Copy link

carlj-w commented May 17, 2019

I fully agree that noClock is a nice convenience for the user. However, what I do not see is the benefit from the simulation perspective.

In contrast there is a clear benefit for noClock when dealing with different Basepartitions.

So for me the question is, whether we should open noClock up to allow for different Basepartitions or introduce a different operator because there is considerable value in that functionality with respect to what sample(hold(u)) offers.

Yes, I also don't see any benefit from the simulation perspective. I don't think there is one, nor do I believe there needs to be one.

I would propose that this would be a differently named operator. Using noClock for both subclock conversion and baseclock conversion can cause problems when doing the clock partitioning. E.g., if you are doing base clock partitioning and get a noClock, is that a baseclock-noClock (where you would ignore the argument) or a subclock-noClock (where you get incidences from the argument)?

I'm unsure if (2) would work even with an implicit delay, since the two base-clocks may not only tick at the same "time" - but also at slightly different times close together (that was after all one of the issues that synchronous extension would solve).

Yes, it might be problematic.

@miscco
Copy link
Collaborator

miscco commented May 17, 2019

So lets summarize that we would like a different operator that does the same as noClock but between different Basepartitions.

At the same time we definitely need to considerably impove the wording of the specification with regard to what last tick etc actually mean. Also what do we do if we get some algebraic loop between the Subpartitions.

@carlj-w
Copy link

carlj-w commented May 17, 2019

So lets summarize that we would like a different operator that does the same as noClock but between different Basepartitions.

Well, I am not sure if I want it. What are the use cases? This should be a separate ticket, probably an MCP?

At the same time we definitely need to considerably impove the wording of the specification with regard to what last tick etc actually mean.

Agreed, the wording needs to be improved.

Let's start with this:

The clock of y = noClock(u) is always inferred.

This seems ok, since the second sentence starts out with basically saying that the inferred clock is the clock of y.

At every tick of the clock of y, the operator returns the value of u from the last tick of the clock of u.

I propose that the above is changed to:

At every tick of the clock of y, the noClock(u) returns the value of u from the most recent tick of the clock of u, which might coincide with the current tick of the clock of y.

Last sentence in the current spec:

If noClock(u) is called before the first tick of the clock of u, the start value of u is returned.

Keep as it is.

Add the following (as non-normative? I don't know the policy on this):

noClock provides an interface between sub partitions without needing to specify the sampling relation between the sub partitions.

This can be contrasted with calling e.g. y = subSample(x), which doesn't tell you exactly what the relation between the clock of y and the clock of x is, but it does tell you that the clock of y is at most as fast as the clock of x.

So the full new text would be:

The clock of y = noClock(u) is always inferred. At every tick of the clock of y, noClock(u) returns the value of u from the most recent tick of the clock of u, which might coincide with the current tick of the clock of y. If noClock(u) is called before the first tick of the clock of u, the start value of u is returned. noClock provides an interface between sub partitions without needing to specify the sampling relation between the sub partitions.

@HansOlsson
Copy link
Collaborator Author

HansOlsson commented May 17, 2019

Comment on proposed full new text:

The clock of y = noClock(u) is always inferred. At every tick of the clock of y, noClock(u) returns the value of u from the most recent tick of the clock of u, which might coincide with the current tick of the clock of y. If noClock(u) is called before the first tick of the clock of u, the start value of u is returned. noClock provides an interface between sub partitions without needing to specify the sampling relation between the sub partitions.

I would change last sentence to (changes in italics) "The operator noClock provides an interface between sub-clock partitions without having to specify the sampling relation between the sub-clock partitions."

And I thought I had pressed "comment" on this:

So lets summarize that we would like a different operator that does the same as noClock but between different Basepartitions.

I think we can wait with that; until we see a clear need - since it seems to be problematic to accomplish it, and it goes counter to the idea of synchronous models.

At the same time we definitely need to considerably impove the wording of the specification with regard to what last tick etc actually mean.

Yes. The first part is just to clarify the existing operators; both to clarify their behavior and the restrictions on using them (in particular for noClock).

@miscco
Copy link
Collaborator

miscco commented May 17, 2019

I think we should at the beginning of the whole Chapter define what we mean by

last tick of the clock of u

as that is the foundation of everything and should be consistent between all partitions (including linked rational interval clocks).

I think that you description for noClock is a good start

At every tick of the clock of y, the noClock(u) returns the value of u from the most recent tick of the clock of u, which might coincide with the current tick of the clock of y.

What I miss is a reference to the ordering of the subpartitions in the case that both tick during the activation of the basepartition. So I would phrase is like this

At every tick of the clock of y, the subclock conversion operators return the value of u from the most recent tick of the clock of u. If the ticks of the clock of y and the clock of u coincide it is required that the evaluation of the subpartition of u is performed before the evaluation of subpartition of y. It is an error if there is no unambiguous ordering of the subpartitions that fullfils this requirement.

We should also consider whether we speak of tick or activation

@miscco
Copy link
Collaborator

miscco commented May 17, 2019

think we can wait with that; until we see a clear need - since it seems to be problematic to accomplish it, and it goes counter to the idea of synchronous models.

I think that this will be a quite common use case where you have a controller that samples with a certain rate without necessarily knowing the sampling rate of some sensor.

The possibility to omit Event-Iteration in these cases seems to be worthwile. However I agree this is something for an MCP

@carlj-w
Copy link

carlj-w commented May 20, 2019

If the ticks of the clock of y and the clock of u coincide it is required that the evaluation of the subpartition of u is performed before the evaluation of subpartition of y. It is an error if there is no unambiguous ordering of the subpartitions that fullfils this requirement.

I don't see a need for this requirement.

@HansOlsson
Copy link
Collaborator Author

If the ticks of the clock of y and the clock of u coincide it is required that the evaluation of the subpartition of u is performed before the evaluation of subpartition of y. It is an error if there is no unambiguous ordering of the subpartitions that fullfils this requirement.

I don't see a need for this requirement.

I see two possibilities without that restriction:

  • Prohibit system of equations involving multiple sub-clocks (Dymola has such check; regardless of noClock - it might exist in the current specification, but I couldn't easily find it), but don't require that the entire sub-clock partitions are ordered. (Technically this implies that we can split sub-clock partition into smaller parts that are either independent or can be ordered.)
  • No restriction at all, so there might be a non-linear system involving multiple sub-clocks.

I see problems with the latter alternative, and such cases can occcur, consider:

model Unnamed
  Clock c=Clock(1, 10);
  Real x,y;
equation
  when subSample(c, 3) then
    x=noClock(y);
  end when;
  when subSample(c, 2) then
    y=noClock(x)/2+sample(time);
  end when;
end Unnamed;

@carlj-w
Copy link

carlj-w commented May 20, 2019

If the ticks of the clock of y and the clock of u coincide it is required that the evaluation of the subpartition of u is performed before the evaluation of subpartition of y. It is an error if there is no unambiguous ordering of the subpartitions that fullfils this requirement.
I don't see a need for this requirement.

I see two possibilities without that restriction:

* Prohibit system of equations involving multiple sub-clocks (Dymola has such check; regardless of noClock - it might exist in the current specification, but I couldn't easily find it), but don't require that the entire sub-clock partitions are ordered. (Technically this implies that we can split sub-clock partition into smaller parts that are either independent or can be ordered.)

Yes, this is a more reasonable restriction than requiring the sub partitions to be ordered.

* No restriction at all, so there might be a non-linear system involving multiple sub-clocks.

I see problems with the latter alternative, and such cases can occcur, consider:

model Unnamed
  Clock c=Clock(1, 10);
  Real x,y;
equation
  when subSample(c, 3) then
    x=noClock(y);
  end when;
  when subSample(c, 2) then
    y=noClock(x)/2+sample(time);
  end when;
end Unnamed;

But this model can be handled. Depending on which subclock ticks, you have different active equations. What are the problems you see?

@HansOlsson
Copy link
Collaborator Author

The problem I see is that if subsample(c, 3) ticks but not subsample(c, 2) then x gets the previous value of y. If subsample(c, 3) does not tick, but subsample(c, 2) ticks then y=previous(x)/2+sample(time); (sort of).
If both subsample tick at the same time then y=y/2+sample(time); => y=2*sample(time).

@carlj-w
Copy link

carlj-w commented May 20, 2019

The problem I see is that if subsample(c, 3) ticks but not subsample(c, 2) then x gets the previous value of y. If subsample(c, 3) does not tick, but subsample(c, 2) ticks then y=previous(x)/2+sample(time); (sort of).
If both subsample tick at the same time then y=y/2+sample(time); => y=2*sample(time).

I agree with this interpretation of the model, but I don't think it's a problem. To me this seems to be something a tool could handle.

@HansOlsson
Copy link
Collaborator Author

I agree with this interpretation of the model, but I don't think it's a problem. To me this seems to be something a tool could handle.

I assume so, even if I don't know how complicated such things can become.
However, to me it would indicate a problem with the design - and thus even if it could be handled, I think it is (at least for now) better to not allow equation-systems mixing different sub-clocks.

@miscco
Copy link
Collaborator

miscco commented May 21, 2019

I want to mention that the requirement I wrote was about ambiguity

If the ticks of the clock of y and the clock of u coincide it is required that the evaluation of the subpartition of u is performed before the evaluation of subpartition of y. It is an error if there is no unambiguous ordering of the subpartitions that fullfils this requirement.
I don't see a need for this requirement.

One could simply extend the problem from Hans win another term:

model Unnamed
  Clock c=Clock(1, 10);
  Real x,y;
equation
  when subSample(c, 3) then
    x=noClock(y)+sample(time);;
  end when;
  when subSample(c, 2) then
    y=noClock(x)/2+sample(time);
  end when;
end Unnamed;

Here it is completely unclear how to order the equations. Therefore if we allow such models, the qualitative result of the computation would be tool dependent which is something we should avoid.

@carlj-w
Copy link

carlj-w commented May 21, 2019

model Unnamed
  Clock c=Clock(1, 10);
  Real x,y;
equation
  when subSample(c, 3) then
    x=noClock(y)+sample(time);;
  end when;
  when subSample(c, 2) then
    y=noClock(x)/2+sample(time);
  end when;
end Unnamed;

Here it is completely unclear how to order the equations. Therefore if we allow such models, the qualitative result of the computation would be tool dependent which is something we should avoid.

I don't think It is unclear.
When both subSample(c, 3) and subSample(c, 2) tick you solve:

x=noClock(y)+sample(time);
y=noClock(x)/2+sample(time);

for x and y.
When only subSample(c, 3) ticks you solve:

x=noClock(y)+sample(time);

for x.
When only subSample(c, 2) ticks you solve

y=noClock(x)/2+sample(time);

for y.

However, to me it would indicate a problem with the design - and thus even if it could be handled, I think it is (at least for now) better to not allow equation-systems mixing different sub-clocks.

Are you referring to the design of Modelica or the design of the model (or both)? I can see it being a problem with the design of the model, but also that it could be a desired feature (e.g., once in a while you solve a larger system of equations in a controller).

We could make it a quality of implementation issue.

@HansOlsson
Copy link
Collaborator Author

Are you referring to the design of Modelica or the design of the model (or both)?

A problem with the design of the model.

I can see it being a problem with the design of the model, but also that it could be a desired feature (e.g., once in a while you solve a larger system of equations in a controller).

i agree that it in rare cases it might be desired (but I haven't seen such realistic use-cases), but to me it is much more likely that it is an error and thus it is better for users to get a diagnostics message.

@miscco
Copy link
Collaborator

miscco commented May 21, 2019

The problem is that it is not clear what you solve if both are active

x=noClock(y)+sample(time);
y=noClock(x)/2+sample(time);

or

y=noClock(x)/2+sample(time);
x=noClock(y)+sample(time);

Depending on which one you choose the result of the model is qualitative different.

@carlj-w
Copy link

carlj-w commented May 21, 2019

The problem is that it is not clear what you solve if both are active

x=noClock(y)+sample(time);
y=noClock(x)/2+sample(time);

or

y=noClock(x)/2+sample(time);
x=noClock(y)+sample(time);

Depending on which one you choose the result of the model is qualitative different.

It's a system of equations.

@HansOlsson HansOlsson added this to the ModelicaSpec3.5 milestone Feb 12, 2020
@HansOlsson
Copy link
Collaborator Author

Trying to summarize this one so that we can get a decision at some point.
There are two parts:

  • Should we forbid noClock between different base-clocks. (Yes/No). It seem forbidding is favored.
  • Should we forbid system of equations between different sub-clocks. (Yes/No). Personally I would prefer forbidding it, as it is likely a modeling error - and add it later if necessary.

@HansOlsson
Copy link
Collaborator Author

Trying to summarize this one so that we can get a decision at some point.
There are two parts:

  • Should we forbid noClock between different base-clocks. (Yes/No). It seem forbidding is favored.
  • Should we forbid system of equations between different sub-clocks. (Yes/No). Personally I would prefer forbidding it, as it is likely a modeling error - and add it later if necessary.

Agreement on forbidding both at phone meeting.
We can always add the system of equations spanning different sub-clocks later if needed.

@HansOlsson HansOlsson added the decided A decision has been made (label added before the spec is changed) label Sep 15, 2020
HansOlsson added a commit to HansOlsson/ModelicaSpecification that referenced this issue Sep 24, 2020
Closes modelica#2365

Specifically:
* Forbids noClock between different base-clocks.
* Forbids system of equations between different sub-clocks.
HansOlsson added a commit that referenced this issue Oct 5, 2020
Closes #2365

Specifically:
* Forbids noClock between different base-clocks.
* Forbids system of equations between different sub-clocks.

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
decided A decision has been made (label added before the spec is changed)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants