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

Simplify synchronous features #3240

Merged
merged 17 commits into from
Nov 23, 2022
Merged

Simplify synchronous features #3240

merged 17 commits into from
Nov 23, 2022

Conversation

henrikt-ma
Copy link
Collaborator

Initiating empty PR as decided in today's web meeting.

@henrikt-ma henrikt-ma changed the base branch from master to MCP/0031 September 13, 2022 12:40
@HansOlsson HansOlsson added the MCP0031 Base Modelica and MLS modularization (MCP-0031) label Sep 13, 2022
@olivleno
Copy link
Collaborator

Web Meeting (Hans, Henrik, Gerd, Martin, Oliver)

It will drastically simplify the language if the system is partitioned in clocked partitions during flattening.

A clocked partition needs to be declared in Flat Modelica somehow. This could be done by:

  1. currently there is only one model, but we could have model as a container for multiple partitions
  2. use the when clocked
  3. Have a new construct to define partitions.

Discussion:
To 1.) It would be weird to have references to variables in other models, which would be the case for interface variables or to records being used in multiple partitions.

to 2.)
Mainly works for equations, but fails for variables and algorithms.
Also not clear how to handle subclocks. Requires to make the base partition explicit.

To be continued in next meeting.

@olivleno
Copy link
Collaborator

Web Meeting (Hans, Henrik, Gerd, Martin, Oliver)

package 'p'
model 'p'
  Real 'y';
  Real 'x' = hold('y');
  partition 
    Clock baseClock=Clock(1e-3);

    subpartition
      Clock subClock=expr(baseClock);
      equation
       'y'=previous('x');  //or sample or noClock to indicate value of the current clock tick of the other subpartition
      algorithm

      annotation(solverMethod="ExplicitEuler");
    end subpartition;

    subpartition
      equation
          ...
      algorithm
    end subpartition;
  end partition;
  partition
    ...
  end partition;
end 'p';
end 'p';

Open issue:
How to guard variables from another subpartition?

  • Does noClock imply an acyclic ordering of subpartitions?

@henrikt-ma
Copy link
Collaborator Author

We should also keep parameters and constants in mind when defining how to infer the set of variables belonging to a subpartition.

@olivleno
Copy link
Collaborator

olivleno commented Sep 30, 2022

Web Meeting (Hans, Henrik, Gerd, Martin, Oliver)

noClock() is not allowed to create cyclic dependencies between subpartitions to be fixed in full Modelica.
Create an issue [Hans].

In Flat Modelica we don't want to have cyclic dependencies due to noClock() between subpartitions.
The assumption is that this will be the case in full Modelica.

Using previous() to indicate value of the current clock tick of the other subpartition might lead to wrong semantics.

Example

model M
  Real x;
  Real baseVar, cVar1, cVar2, cVar3;
  Real mixedVar1;
equation
  der(x)=1;
  baseVar = sample(time, Clock(1));
  cVar1 = subSample(baseVar, 2);
  cVar2 = superSample(baseVar, 0);
  cVar3 = superSample(cVar1, 4);
  mixedVar1 = cVar2+cVar3;
end M;
model 'M'
  Real 'x';
  Real 'baseVar', 'cVar1', 'cVar2', 'cVar3';
  Real 'mixedVar1';
equation
  der(x)=1;
partition 
  Clock baseClock=Clock(1);

  subpartition
    Clock subClock0=baseClock;
  equation
     baseVar = sample(time);  //use sample() instead of noClock() because time belongs to the continuous part

  subpartition
    Clock subClock1=subSample(baseClock, 2);
  equation
      cVar1 = noClock(baseVar);  //to indicate baseVar coming from another partition

  subpartition
    Clock subClock2=superSample(subSample(baseClock, 2), 8);
  equation
    cVar2 = noClock(baseVar);
    cVar3 = noClock(cVar1);  //using previous() instead would give wrong results by adding a delay
                                             //using sample() instead would not be legal
    mixedVar1 = cVar2+cVar3;
partition
 Clock baseClock2=Clock(1.1);
...
end M;

noClock() shall be used to guard variables from other subpartitions.
sample() shall be used to guard variables from continuous time partition.
time is a special case, but is considered as variable from the continuous time partition.

The order of the subpartitions is arbitrary, are not required to be in the order of their dependency.
Revisit this question next meeting.

@olivleno
Copy link
Collaborator

olivleno commented Oct 7, 2022

Web Meeting (Hans, Henrik, Gerd, Martin, Oliver)

Discussion of the above approach:

  • clocked sample() will have a different syntax than in full Modelica, as the clock is already given by the partition.
  • The partition has no "end". This requires the continuous part to be defined at the top. Is this intended?
  • Will look like equation.
  • There are many constructs in Modelica using end
  • Shall it be allowed to define a clocked partition inside an equation section?
  • Do we want all clocked partitions to be defined at the lower end?

Idea to avoid end partition:

model 'M'
  Real 'x';
  Real 'baseVar', 'cVar1', 'cVar2', 'cVar3';
  Real 'mixedVar1';

equation
  der(x)=1;  //should come first for readability and easier parsing

  partition baseClock=Clock(1)
    subpartition subClock0=baseClock
    equation
       baseVar = sample(time);  //use sample() instead of noClock() because time belongs to the continuous part
    end subClock0;

    subpartition subClock1=subSample(baseClock, 2) 
    equation
      cVar1 = noClock(baseVar);  //to indicate baseVar coming from another partition
    end subClock1;

    subpartition subClock2=superSample(subSample(baseClock, 2), 8)
    equation
      cVar2 = noClock(baseVar);
      cVar3 = noClock(cVar1);  //using previous() instead would give wrong results by adding a delay
                                             //using sample() instead would not be legal
      mixedVar1 = cVar2+cVar3;
    end subClock2;
  end baseClock;

  partition baseClock2=Clock(1.1)
  ... 
  end baseClock2;


end M;

Alternative 1: Clocks as component
pros:

  • Normal component declaration for something that has (tool specific) simulation results.
  • Natural for the continuous time partition to go first.
  • Less indentation.
  • "Clock" is naturally associated with something concrete.
  • No need to pick a keyword to close the opening lines for partitions.
  • Better readability in terms of cleaner separation of partitions and subpartitions with a simple opening line

Alternative 2: Begin Clock - end Clock
pros:

  • Avoid extra declaration of clock component.

Poll
Alternative 1: Henrik, Gerd, Oliver
Alternative 2:
abstain: Hans, Martin

Decision:
Go with alternative 1, try and see if there any issues popping up during test implementation phase.

Revisit this question of the order subpartition.
Alternative 1 order of subpartitions is arbitrary
pros:

  • fewer rules need less checking.
  • manually coded may not be aware of this constraint.
  • in Modelica the order of equations and dependency between components is inferred.

Alternative 2: required to be in the order of their dependency.
pros:

  • Use the results of the dependency analysis, which has to be done anyways.
  • We could start with being more restrictive and relax the rules later.
  • Easier to report this as an error.

To be continued.

@olivleno
Copy link
Collaborator

Web Meeting (Hans, Henrik, Gerd, Martin, Oliver)

Henrik: For equations it is not possible to define an order, but there are dependencies between subpartitions that must not be cyclic. This makes it a different case.
Reporting such type of error will be much easier to report to the user.

Oliver: If the execution order is determined by the tool based on the dependencies, then it's different than an algorithm.
The flat Modelica generating tool can dump the code in a specific order for better readability.

Henrik: When the user knows that subpartitions have to be in order will help the understanding of execution at a tick. If it's only a recommendation and not stricty required by the specification, then the user cannot rely on it and derive it by himself to understand the execution order.

Gerd: Agree that error messages will be much easier to comprehend.

Hans: Will need some more thinking to conclude.

Henrik: This could be part of the prototyping.

Gerd: What will be the error message, when generating flat Modelica fails because no order can be derived in case of a cyclic dependency.

Henrik: Flat Modelica can be the way to explain why it was not possible or where to problem occurred.

Option A: Order of subpartitions is strictly required.
Option B: Order of subpartitions is arbitrary.
Option C: Subpartitions are recommended to be in order, but order is not required.

Cyclic dependencies are always reported as error.

Poll
A: Hans, Henrik, Gerd
abstain: Martin, Oliver

Decision: To go forward with option A and revisit this topic based on the experience gained in prototyping phase.

  • Adding the details to the PR [Henrik]
  • Review the PR [all]

henrikt-ma and others added 5 commits October 19, 2022 20:41
Co-authored-by: Hans Olsson <HansOlsson@users.noreply.github.com>
Also including solverMethod specification, and will make a comment in the PR to drag attention to this part.
> _sub-partition_ →\
> &emsp; **subpartition** _comment_\
> &emsp; _clock-clause_ **;**\
> &emsp; ( **solverMethod** _STRING_ **;** )?\
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We hadn't settled how to specify solver method, so I added this as a starting point.

Copy link
Collaborator

@olivleno olivleno Oct 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Web Meeting: (Hans, Henrik, Gerd, Martin, Oliver)

Subpartitions do not necessarily require a solver method.
In full Modelica there is no natural place to define a solver method for subpartitions. This makes it look a bit ugly and a risk of conflicting definitions.

Gerd: What I don't like is that the solverMethod looks like a declaration. Maybe we could move it ahead of clock-clause.

Henrik: Keyword 'discretization' would look better as a keyword together with partition/subpartition than the lower camel case 'solverMethod'.
If we swop these two lines then we avoid looking like a declaration.
Dropping the semicolon, but having encourages declaration on a separate line, which improves readability as we had discussed previously.

Oliver: solverMethod is a well established term.

Gerd: Could it remain a named argument?

Henrik: Technically the solverMethod is not associated with the clock but with the subpartition. This could not be expressed in a better way in full Modelica.

Examples:

subpartition "Alternative A" 
    Clock _subClock0 = subSample('baseClock', 2);
    solverMethod "ImplicitEuler" annotation(tolerance=1e3);
    ...
subpartition "Alternative B"
    solverMethod "ImplicitEuler" annotation(tolerance=1e3);  //Better place for something that is not a component declaration
    Clock _subClock0 = subSample('baseClock', 2);
    ...
subpartition "Alternative C"
    discretization "ImplicitEuler"  annotation(tolerance=1e3);
    Clock _subClock0 = subSample('baseClock', 2);
    ...
subpartition "Alternative D" discretization "ImplicitEuler" 
    annotation(tolerance=1e3)  // Keeping everything together
    Clock _subClock0 = subSample('baseClock', 2);
    ...
subpartition(discretization(tolerance=1e3)="ImplicitEuler") "Alternative E" 
    Clock _subClock0=subSample('baseClock', 2);
    ...
subpartition subPart1 "Alternative F" 
    extends basepartition(
      solverMethod="ImplicitEuler", tolerance=1e3,
      subClock=subSample('baseClock', 2));
    ...
end subPart1;
subpartition subE2(
  clock=subsample('baseClock',2),
   solverMethod="ImplicitEuler", tolerance=1e3 "Alternative G" 
subpartition "Alternative H" 
    Clock _subClock0 = subSample('baseClock', 2);
    String solverMethod = "ImplicitEuler" annotation(tolerance=1e3);

Henrik: There might be the need for more sophisticated attributes for solver methods in the future. This could be done by:
solverMethod "ImplicitEuler"(tolerance=1e3);
We have an annotation on the subpartition and could also have an annotation on the solverMethod as well.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Web Meeting: Hans, Henrik, Gerd, Martin, Oliver
Revised options above by adding tolerance to all of them.
Added Alternative H.

Discussion to be concluded in next meeting.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me (Hans):

Most preferred: G (Also called E2 above)
Good: F
Acceptable: H

A, B, C, D all seem odd, as if it were a declaration of a Modelica variable discretization with description-string "ImplicitEuler" and we forgot its type (a mistake that some people do). E has the problem that clock and solverMethod are treated differently, but if really really needed I could accept it.

On a separate note: in general I believe that Rosenbrock methods are a better solution than ImplicitEuler with tolerance; thus I don't think the tolerance will be used much (and our experience in Dymola is that it isn't just a matter of one tolerance).

Another option for sophisticated attributes would be solverMethod(tolerance=1e3)="ImplicitEuler".

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

subpartition (
  solverMethod="ImplicitEuler", tolerance=1e-3) "Alternative I" 
  Clock _subClock0 = subsample('baseClock',2);

@HansOlsson
Copy link
Collaborator

HansOlsson commented Oct 24, 2022

An interesting extra is:

model M
  Real x;
  Real baseVar, cVar1, cVar2, cVar3, cVar5;
  Real mixedVar1;
equation
  der(x)=1;
  baseVar = sample(time, Clock(1));
  cVar1 = subSample(baseVar, 2);
  cVar2 = superSample(baseVar, 0);
  cVar3 = superSample(cVar1, 4);
  mixedVar1 = cVar2+cVar3;
  cVar5 = baseVar+superSample(cVar1);
end M;

The complication is cVar5 will tick at the same times as baseVar, but it needs to be a separate subclock-partition:

model 'M'
  Real 'x';
  Real 'baseVar', 'cVar1', 'cVar2', 'cVar3', 'cVar5';
  Real 'mixedVar1';
equation
  der(x)=1;
partition 
  Clock baseClock=Clock(1);

  subpartition
    Clock subClock0=baseClock;
  equation
     'baseVar' = sample(time);  //use sample() instead of noClock() because time belongs to the continuous part

  subpartition
    Clock subClock1=subSample(baseClock, 2);
  equation
      'cVar1' = noClock('baseVar');  //to indicate baseVar coming from another partition

  subpartition
    Clock subClock2=superSample(subSample(baseClock, 2), 8);
  equation
    'cVar2' = noClock('baseVar');
    'cVar3' = noClock('cVar1');  //using previous() instead would give wrong results by adding a delay
                                             //using sample() instead would not be legal
    'mixedVar1' = 'cVar2'+'cVar3';
  subpartition
    Clock subClock3=baseClock;
  equation
    'cVar5' = noClock('baseVar')+noClock('cVar1');
    // The order must be subClock0->subClock1->subClock2->subClock3
    // even if subClock0 and subClock3 tick at the same time
partition
 Clock baseClock2=Clock(1.1);
...
end M;

There are two implications:

  • There might be additional sub-clock partitions to ensure that everything can be sorted.
  • There might be additional calls of noClock to separate them (even though it was just a variable reference in the original model).

As far as I can see loops between different (sub-)clock partitions should be forbidden, will see how to ensure that for the normal spec.

This is still being discussed, but the original proposal was not popular at all.
This matches how annotations are placed in other parts of the grammar.
This shows that we don't really need any new syntax for handling the choice of solver method, at the cost of having this important information hidden away in an annotation.
As suggested by Hans.

Co-authored-by: Hans Olsson <HansOlsson@users.noreply.github.com>
@olivleno
Copy link
Collaborator

olivleno commented Nov 2, 2022

Web Meeting: Hans, Henrik, Gerd, Martin, Oliver

Option1: Extend the syntax to capture the solver method.
Option1b: Pick one of the already discussed solutions extending the syntax.
Option2: Use annotation to capture the solver method.

Poll
in favor of 1: Martin, Hans
in favor of 2: Henrik, Gerd
abstain: Oliver

Decision:
Revisit the syntax proposals and time box the discussion. Make a poll on syntax proposal or annotation at the end of next meeting.

@olivleno
Copy link
Collaborator

Web Meeting: Hans, Henrik, Gerd, Martin, Oliver

Currently missing in the design:

  • two subpartition using the same clock

This could be solved by either:

  • subpartition clock declared as an alias of an earlier subpartition's clock
  • declare all clocks at the top

Do we need a name for a subclock?

Should it be visible to which basepartition a subclock belongs?

  • Yes.

Highest chance to with all implementation has the approach of having all subclock declarations at the top of basepartition.

According to the specification there is only one basepartition.

Probably some prototyping needed.
It would be draw back if we had to reconstruct information that gets lost after finding subpartitions.

Conclusion:

Clock mySubClock = subsample('baseClock',2),

subpartition (
  clock=mySubClock,
   solverMethod="ImplicitEuler", tolerance=1e-3) "Alternative J" 

Parenthesis after a partition is unusual for Modelica, compared with:

subpartition "Alternative K" 
  annotation(clock=mySubClock,
   solverMethod="ImplicitEuler", tolerance=1e-3);

But it's undesired to have crucial information in an annotation.

The annotation is often cluttered with graphical and other stuff.

Poll decide between option J and K
Option J in favor: Hans, Martin, Gerd, Oliver
Option K favor: Henrik, Gerd

Decision:
in favor of J

@olivleno
Copy link
Collaborator

Web Meeting (Hans, Henrik, Gerd, Martin, Oliver)

proposed design approved

@olivleno olivleno closed this Nov 23, 2022
@olivleno olivleno reopened this Nov 23, 2022
@olivleno olivleno marked this pull request as ready for review November 23, 2022 07:44
@olivleno olivleno merged commit 5a70185 into MCP/0031 Nov 23, 2022
@olivleno olivleno deleted the MCP/0031+synchronous branch November 23, 2022 07:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
MCP0031 Base Modelica and MLS modularization (MCP-0031)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants