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

Blocks.Math.Mean: Order of when-equations relevant => rewrite as algorithm! #963

Closed
modelica-trac-importer opened this issue Jan 14, 2017 · 11 comments
Assignees
Labels
bug Critical/severe issue L: Blocks Issue addresses Modelica.Blocks
Milestone

Comments

@modelica-trac-importer
Copy link

Reported by naehring on 15 Jan 2013 12:16 UTC
If one swaps the evaluation order of the two equations

y = if not yGreaterOrEqualZero then f*x else max(0.0, f*x);
reinit(x, 0);

in Blocks.Math.Mean the result would always be y=0.
But, the specification requires that the order of equations is irrelevant.
Therefore, the when-block should be located within an algorithm. An alternative would be to replace x by pre(x) in the assignment to y. I.e.:

y = if not yGreaterOrEqualZero then f*pre(x) else max(0.0, f*pre(x));
reinit(x, 0);

Migrated-From: https://trac.modelica.org/Modelica/ticket/963

@modelica-trac-importer modelica-trac-importer added bug Critical/severe issue L: Blocks Issue addresses Modelica.Blocks labels Jan 14, 2017
@modelica-trac-importer
Copy link
Author

Comment by hansolsson on 16 Jan 2013 10:34 UTC
The problem is that reinit is not well-defined, see ticket #578

If reinit was defined as proposed in that ticket (i.e. the reinit in equations is done after evaluating the model) then both orderings will give the same result.

@modelica-trac-importer
Copy link
Author

Comment by naehring on 16 Jan 2013 12:16 UTC
The Modelica specification says in Section "8.3.6 reinit":

The reinit operator does not break the single assignment rule, because reinit(x,expr) in equations evaluates expr to a value (value), then makes the previously known variable x unknown and introduces the equation “x = value”. In algorithms reinit(x,expr) evaluates expr to a value (value), and then performs the assignment “x := value”.

Therefore,

y = if not yGreaterOrEqualZero then f*x else max(0.0, f*x);
reinit(x, 0);

is conceptually replaced by:

y = if not yGreaterOrEqualZero then f*x else max(0.0, f*x);
x = 0;

Note, that I've read #578 including the associated comments before writing this comment.

Shifting the reinits to the end of the event step evaluation would just break the freedom of the tool to sort the equations. Up to now this freedom is guaranteed by the specification.

To access the left limit of the continuous states within when-blocks one has the pre-operator.

@modelica-trac-importer
Copy link
Author

Comment by sjoelund.se on 16 Jan 2013 12:32 UTC
Can you really do that for continuous states with pre?

3.7.3
... if the following three conditions are fulfilled simultaneously ...
(b) y is a discrete-time expression

@modelica-trac-importer
Copy link
Author

Comment by anonymous on 16 Jan 2013 12:45 UTC
See section 3.8.3: The statements inside a when clause are discrete.
And per definition: pre(...) returns the “left limit” y(tpre) of variable y(t) at a time instant t.

@modelica-trac-importer
Copy link
Author

Comment by naehring on 16 Jan 2013 12:59 UTC
Comment 4 was written too hasty. Please, replace "statements" by "expressions".
Sorry.

@modelica-trac-importer
Copy link
Author

Comment by hansolsson on 16 Jan 2013 13:30 UTC
Note that the alterative with pre(x) would continue to work regardless of the clean-up of the when-statements (and predictably as well). Thus it might be safe to perform that change and then continue with the clean-up of when.

As for that: when-clauses are defined one-by-one so that

when ... then
  y = if not yGreaterOrEqualZero then f*x else max(0.0, f*x);
  reinit(x, 0);
end when;

is according to 8.3.5.1 split into two equations, the first is conceptually:

y=if edge(...) then (if not yGreaterOrEqualZero then f*x else max(0.0, f*x)) else pre(y);

The second is the problematic one. The definition states that we evaluate the 2nd argument to reinit - and then change the equations and resort - and it is not clear exactly how this sorting works (and having multiple sorting of all equations seems expensive at least). The first proposal in #578 was to avoid this additional sorting, but unfortunately it changed behaviour of models and was not as well-defined as intended.

The reason that resorting matter is that we could have:

y=2*x
when ... then
  reinit(x, y);
end when;

The resorting semantic seem to imply that we evaluate it as follows:

y:=2*x;
if edge(...) then
   val_x:=y;
   // Resorted equations:
   x:=val_x;
   y:=2*x; // Again!
end if;

Note that with resorting there is not a system of equations since val_x is evaluated - and then x is made unknown. The first proposal in #578 would change this to be a system of equations.

The idea with delaying the reinitialization was to still allow the equations to be sorted freely, but delay the effect of the new value until the next event iteration. The benefits are that the models behave consistently, the overhead is low, and models that are in use continue to work as before.

It will require an additional event iteration for reinit, but since we want to avoid using reinit that should not be a major issue.

For algorithms no sorting is done (neither now - nor in the proposal); and I now realize that this might also gives unpredictable result if any equation/algorithm depend on the reinitialized variable. As long as the use is outside of when-clauses an additional event-iteration will fix it (exactly as for other reinit).

For algorithms one possibility to get predictable result would be to update the variable inside the algorithm - but delay the effect outside of the algorithm until the end of the model evaluation.

@modelica-trac-importer
Copy link
Author

Comment by otter on 16 Jan 2013 13:57 UTC
Replying to [comment:6 hansolsson]:

The first proposal in #578 was to avoid this additional sorting, but unfortunately it changed behaviour of models and was not as well-defined as intended.

The reason that resorting matter is that we could have:

y=2\*x
when ... then
reinit(x, y);
end when;

The resorting semantic seem to imply that we evaluate it as follows:

y:=2\*x;
if edge(...) then
val_x:=y;
// Resorted equations:
x:=val_x;
y:=2\*x; // Again!
end if;

Note that with resorting there is not a system of equations since val_x is evaluated - and then x is made unknown. The first proposal in #578 would change this to be a system of equations. 

I am not able to completely follow. My (first) proposal in #578 is to (conceptually) rewrite the equation to:

y=2*x
x = if edge(condition) then y else fromIntegrator(id_x)

Meaning that x is either computed in the reinit-clause, or the (known) value provided by the integrator is used. Sorting the above equation results in an algebraic loop, since y depends on x and x depends on y. Since algebraic loops are not allowed between a when-clause and a continuous-time part, the model would be rejected (the model is wrong).

@modelica-trac-importer
Copy link
Author

Comment by hansolsson on 16 Jan 2013 14:08 UTC
Replying to [comment:7 otter]:

Replying to [comment:6 hansolsson]:

...

I agree Martin for the first proposal in #578, it is just that if we follow the resorting semantics from 8.3.6 as quoted above there would not be an algebraic loop; instead the equations would be duplicated.

@modelica-trac-importer
Copy link
Author

Comment by otter on 17 Jan 2013 11:29 UTC
Fixed in 39bbd4b:

Changed the equations by using pre(x). Therefore, the model is correct, independently
how reinit is defined (whether it takes immediate effect, or at model end).

@modelica-trac-importer
Copy link
Author

Comment by anonymous on 17 Jan 2013 13:35 UTC
Now that this is fixed set milestone to 3.2.1 please.

@modelica-trac-importer
Copy link
Author

Modified by otter on 17 Jan 2013 13:36 UTC

@modelica-trac-importer modelica-trac-importer added this to the MSL3.2.1 milestone Jan 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Critical/severe issue L: Blocks Issue addresses Modelica.Blocks
Projects
None yet
Development

No branches or pull requests

2 participants