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

Change so that all variables in initial algorithms are treated the same #3562

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

HansOlsson
Copy link
Collaborator

Closes #3472

@HansOlsson
Copy link
Collaborator Author

Note this is already how it works in Dymola.

I'm not aware of any tool that implemented the semantic that lead to a loop for any variable with a when-clause also assigned in an initial algorithm.

@casella
Copy link
Collaborator

casella commented Aug 13, 2024

As I wrote in #3472, this is also how I understand it works in OMC already.

@phannebohm, @kabdelhak, can you please confirm?

@@ -34,11 +34,11 @@ \subsection{An Algorithm in a Model}\label{execution-of-an-algorithm-in-a-model}
\item
A continuous-time variable is initialized with the value of its \lstinline!start!-attribute.
\item
A discrete-time variable \lstinline!v! is initialized with \lstinline!pre(v)!.
A discrete-time variable \lstinline!v! in a non-initial algorithm is initialized with \lstinline!pre(v)!.
Copy link
Collaborator

@henrikt-ma henrikt-ma Aug 27, 2024

Choose a reason for hiding this comment

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

Aren't we talking any algorithm when being part of the initialization problem? To me, an initial algorithm sounds lika an initial algorithm. Hence:

Suggested change
A discrete-time variable \lstinline!v! in a non-initial algorithm is initialized with \lstinline!pre(v)!.
Within transient analysis, a discrete-time variable \lstinline!v! is initialized with \lstinline!pre(v)!.

Copy link
Collaborator Author

@HansOlsson HansOlsson Sep 11, 2024

Choose a reason for hiding this comment

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

No, as indicated that change would be incorrect and break things. The initial algorithm mean initial algorithm; the term "initial algorithm" was already used in this list with that meaning.

\item
If at least one element of an array appears on the left hand side of the assignment operator, then the complete array is initialized in this algorithm section.
\item
A parameter assigned in an initial algorithm, \cref{initialization-initial-equation-and-initial-algorithm}, is initialized with the value of its \lstinline!start!-attribute.
In an initial algorithm, \cref{initialization-initial-equation-and-initial-algorithm}, any variable (including a parameter) is initialized with the value of its \lstinline!start!-attribute.
Copy link
Collaborator

@henrikt-ma henrikt-ma Aug 27, 2024

Choose a reason for hiding this comment

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

Similarly:

Suggested change
In an initial algorithm, \cref{initialization-initial-equation-and-initial-algorithm}, any variable (including a parameter) is initialized with the value of its \lstinline!start!-attribute.
Within the initialization problem \cref{initialization-initial-equation-and-initial-algorithm}, any variable (including a parameter) is initialized with the value of its \lstinline!start!-attribute.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, as indicated that change would be incorrect and break things.

Copy link
Collaborator

@henrikt-ma henrikt-ma left a comment

Choose a reason for hiding this comment

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

I still haven't had time to test implement, but I believe we want the new initialization to also apply to a "non-initial" algorithm when treated as part of the initialization problem. Is this what the tools with test implementations currently do?

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

Check what happens if a normal algorithm ends up in an initial system of equations (seems only problematic if there is a when-clause for the variable as well).

@HansOlsson
Copy link
Collaborator Author

At first I thought that the proposed changes of this PR only seemed relevant for obscure cases. However, investigating more the conclusion is that it would be a breaking change with unknown consequences.

The issue would occur if we have a discrete variable assigned in a when-equation (see later for when-algorithm) and also assigned in a normal algorithm.

That would mean something like:

model M1
  ... x, y;
equation
  when sample(1, 1) then
     x=integer(time);
  end when;
algorithm
   // Should x start at pre(x) or start initially?
   if (...) then
    x:=y;
   end if;
end M1;

The idea is here to solve the algorithm for y. However, if the type of the variables is Integer the problem is that such integer equations aren't guaranteed to work (only very simple integer equations are supported), and for Real the algorithm would attempt to assign a continuous-time value to a discrete variable which is not legal.

However, for an algorithm we could have a case such as:

model M2
  Real x;
  Real z(start=-100, fixed=true);
algorithm
  // initialize x to pre(x) or start(x)
  // initialize z to pre(z) or start(z)
  when ... then
     z:=foo(time, pre(z));
     x:=foo(time, pre(x));
  end when;
initial algorithm
   pre(x)=-100;
end M2;

The description states for z the fixed=true implies that we have an initialization equation pre(z)=-100;
That ensures that if the when-statement is not active z will initialize to -100 (since nothing changes it), and if the when-statement is active it will intialize to f(time, -100).

For x we currently get the same result as for z. However, if we switched initialization of variables in algorithms during initialization in general, then x would not behave the same, but instead always initialize to its "start-value (which is zero as default)" - and it would be impossible to easily replace a start-value for x by an initial equation/initial algorithm giving the same result.

@henrikt-ma
Copy link
Collaborator

At first I thought that the proposed changes of this PR only seemed relevant for obscure cases. However, investigating more the conclusion is that it would be a breaking change with unknown consequences.

Sure, that's why we need a concrete proposal that can be evaluated against existing libraries on our radars.

Copy link
Collaborator

@henrikt-ma henrikt-ma left a comment

Choose a reason for hiding this comment

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

I don't know what kind of review that is expected of me at this time. My impression was that you @HansOlsson would test a couple of variants to see how they work, and then make a concrete suggestion for new algorithm initialization rules based on that. Once there is such a proposal that looks sensible, the rest of us can test implement it to see how it works with the libraries in our test suites.

@HansOlsson
Copy link
Collaborator Author

I don't know what kind of review that is expected of me at this time. My impression was that you @HansOlsson would test a couple of variants to see how they work, and then make a concrete suggestion for new algorithm initialization rules based on that. Once there is such a proposal that looks sensible, the rest of us can test implement it to see how it works with the libraries in our test suites.

The concrete proposal is that only "initial algorithm" is changed, so when the proposed text says "initial algorithm" it only means "initial algorithm". The alternative would have been any algorithm executed during initialization, and as indicated that would cause problems, while only providing a benefit for models that still wouldn't work - so no need to test-implement that.

The change for "initial algorithm" doesn't impact algorithms with "when", and it still keeps "initial equation" and start-attributes in sync - it also seems to be implemented in OpenModelica and Dymola.

Copy link
Collaborator

@casella casella left a comment

Choose a reason for hiding this comment

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

As far as I understand (but I guess we'll need some testing) I agree with @HansOlsson's current proposal in this PR, i.e., replacing

A discrete-time variable \lstinline!v! is initialized with \lstinline!pre(v)!.

with

A discrete-time variable \lstinline!v! in a non-initial algorithm is initialized with \lstinline!pre(v)!.

which leaves open the option of using an explicit initial algorithm to initialize it. However, with this modification it is not clear to me how discrete-time variables would be initialized in initial algorithms, i.e., what value they get before the algorithm starts. I guess it should be the start attribute, and maybe this is already clear because of some other rule regarding algorithms. Maybe we should say it explicitly anyway?

@HansOlsson
Copy link
Collaborator Author

As far as I understand (but I guess we'll need some testing) I agree with @HansOlsson's current proposal in this PR, i.e., replacing

A discrete-time variable \lstinline!v! is initialized with \lstinline!pre(v)!.

with

A discrete-time variable \lstinline!v! in a non-initial algorithm is initialized with \lstinline!pre(v)!.

which leaves open the option of using an explicit initial algorithm to initialize it. However, with this modification it is not clear to me how discrete-time variables would be initialized in initial algorithms, i.e., what value they get before the algorithm starts. I guess it should be the start attribute, and maybe this is already clear because of some other rule regarding algorithms. Maybe we should say it explicitly anyway?

See line 41.

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 this pull request may close these issues.

Implicit dv := pre(dv) in initial algorithm?
3 participants