search

modelica / ModelicaStandardLibrary

Free (standard conforming) library to model mechanical (1D/3D), electrical (analog, digital, machines), magnetic, thermal, fluid, control systems and hierarchical state machines. Also numerical functions and functions for strings, files and streams are included.
https://doc.modelica.org
BSD 3-Clause "New" or "Revised" License
479 stars 169 forks source link

Calling impure function print from equation section in Inverse_sine example #3856

Open MarkusOlssonModelon opened 3 years ago

MarkusOlssonModelon commented 3 years ago

I have found an issue with Modelica.Media.Examples.SolveOneNonlinearEquation.Inverse_sine. It contains a call to Modelica.Utilities.Streams.print directly in the equation section. Since print is an impure function this is not allowed.

I suggest adding an initial equation section and moving the call to print there.

HansOlsson commented 3 years ago

I have found an issue with Modelica.Media.Examples.SolveOneNonlinearEquation.Inverse_sine. It contains a call to Modelica.Utilities.Streams.print directly in the equation section. Since print is an impure function this is not allowed.

Note that there is an exception for printso it is deprecated behavior without any required diagnostics.

I suggest adding an initial equation section and moving the call to print there.

That wouldn't be ideal for me, as it makes the model harder to understand.

Basically x_zerois solved in a normal equation - and then the result printed. Added: I'm not saying that we should leave it as is, but we have to think more about the change.

If the result is printed in an initial equation, you will sort of have to jump back and forth in the code to understand it, and also think more to realize that x_zerodoesn't change after the initialization and thus printing it once suffices.

One possibility would be to declare x_zeroas a parameter with fixed=false and replace the entire equation section by an initial equation section; as it is then ordered in the same way and it is somewhat clear that x_zerois only computed once. Another possibility would be declareparameter x_zero=Modelica.Math.Nonlinear.solveOneNonlinearEquation(...) and then have an initial equation directly following that.

The benefit with the second possibility is that it would correspond to how you normally use solveOneNonlinearEquation that is in algorithmic context where you directly want the value and not in an equation section where it is normally better to just write the actual equation instead (that is equation f_nonlinear(u=x_zero, A=A, w=w, s=-y_zero)=0; )

MarkusOlssonModelon commented 3 years ago

Note that there is an exception for print so it is deprecated behavior without any required diagnostics.

Is that how it is supposed to be interpreted? The way I read the specification print is only excepted from the diagnostics when using the deprecated semantics in a simulation model, not the restrictions on calling impure functions. Except for the function Modelica.Utilities.Streams.print, diagnostics must be given if called in a simulation model.

One possibility would be to declare x_zero as a parameter with fixed=false and replace the entire equation section by an initial equation section; as it is then ordered in the same way and it is somewhat clear that x_zero is only computed once. Another possibility would be declare parameter x_zero=Modelica.Math.Nonlinear.solveOneNonlinearEquation(...) and then have an initial equation directly following that.

I am OK with either.

HansOlsson commented 3 years ago

Note that there is an exception for print so it is deprecated behavior without any required diagnostics.

Is that how it is supposed to be interpreted? The way I read the specification print is only excepted from the diagnostics when using the deprecated semantics in a simulation model, not the restrictions on calling impure functions. Except for the function Modelica.Utilities.Streams.print, diagnostics must be given if called in a simulation model.

I read it as print is 'excepted' from the diagnostics of calling an impure function in a simulation in the 'wrong' place (as print is an impure function that is often called in a simulation model in situations like this); and that is also consistent with my recollection of why it was added.

henrikt-ma commented 3 years ago

I agree with @MarkusOlssonModelon that the specification text as currently written would not allow the impure function print to be called unless it was impure according to the deprecated semantics of not using purity prefixes. However, if it was impure according to the deprecated semantics, it would not have been required to give diagnostics.

That is, a short term fix could be to remove the impure prefix for print, but then the question that remains would be what to do in the long term as we don't want to rely on deprecated behavior.