-d=evaluateAllParameters should not evaluate non-fixed parameters

[Francesco Casella]

Please consider the following simple test case:

model TestParameters
  parameter Real p(fixed = false);
  parameter Real q = 2*p;
initial equation
  p - 2 = 0;
end TestParameters;

The NF handles this nicely in the nightly build corresponding to ​04f1061. One of the three commits on March 11 (I'm not sure which one) apparently broke something, so that the nightly corresponding to ​9656261 gives the following error:

[1] 11:58:17 Translation Notification
[Test: 2:3-2:34]: From here:

[2] 11:58:17 Translation Error
[Test: 3:3-3:25]: Constant p is used without having been given a value.

This test model is perfectly fine, since the parameters without binding equations but having fixed = false are matched to initial equations. This case is apparently not covered in the testuite, so it didn't cause any obvious regression, but it happens to break a number of models at Dynamica, that would otherwise happily run with the new frontend, and don't run with the old frontend on the other hand.

Could you please fix this ASAP and add the test case to the testsuite?

Thanks!

[Per Östlund]

I tried all the recent commits, but I can't reproduce this with any of them. Do you use any special flags besides the obvious -d=newInst? I get this error using -d=evaluateAllParameters, which is perhaps not ideal (maybe non-fixed parameters shouldn't be included), but that's probably been the case since the NF started supporting that flag.

[Andrea Bartolini]

Effectively the problem seems to be the flag evaluateAllParameters, if I remove it, the problem disappears.

I don't remember if the flag was set by me in the OMEdit setting or if it is a default (the flag is set via a chekbox)...

Andrea

[Andrea Bartolini]

In any case, if the use of this flag it is not Modelica legal when some parameters has the modifier fixed=false, maybe better to add a warning in the error message...

[Francesco Casella]

Replying to perost:

I get this error using -d=evaluateAllParameters, which is perhaps not ideal (maybe non-fixed parameters shouldn't be included), but that's probably been the case since the NF started supporting that flag.

We discussed this a bit with Andrea. As we understand, there are two sets of parameters:

• those with fixed = true and binding equations
• those with fixed = false and no binding equations

In principle it is possible to also have parameters with fixed = false and a binding equation (see section 8.6), but this is really not meaningful, and in most cases a leftover of old Dymola code, which originally discarded the binding equation in this case, so we can generate an error in this case

Parameters with fixed = true are matched to their binding equations, and the resulting system of equations should be acyclic after flattening (see section 4.4.3), so that they can be solved explicitly by arranging them in a suitable sequence. Conversely, parameters with fixed = false match with the overall set of initialization equations, so they can only be handled by the backend.

Our proposal is that when you process structural parameters (including the case of -d=evaluateAllParameters) you carry out a dependency analysis in the set of fixed = true parameters, and discard from the evaluation process all those ones that depend directly or indirectly on fixed = false parameters. Obviously, in those cases the evaluation is not possible, but that doesn't make the make the model invalid.

BTW, the old front-end probably does something like this, since it accepts the example case with the intented behaviour.

[Per Östlund]

Replying to Andrea.Bartolini:

Effectively the problem seems to be the flag evaluateAllParameters, if I remove it, the problem disappears.

I don't remember if the flag was set by me in the OMEdit setting or if it is a default (the flag is set via a chekbox)...

Andrea

That flag is not on by default, and is currently interpreted by the NF as "evaluate all parameters, no exceptions". So it's understandable that things break when using the flag, but this is not new behaviour.

Replying to casella:

Our proposal is that when you process structural parameters (including the case of -d=evaluateAllParameters) you carry out a dependency analysis in the set of fixed = true parameters, and discard from the evaluation process all those ones that depend directly or indirectly on fixed = false parameters. Obviously, in those cases the evaluation is not possible, but that doesn't make the make the model invalid.

To me there are two kinds of structural parameters, actually structural parameters and parameters marked as structural because we want to evaluate them even though it's not strictly necessary. Actually structural parameters must be possible to evaluate, so if they depend on non-fixed parameters we should probably give an error even if the non-fixed parameters have bindings. But as you say we should be more careful about marking other parameters as structural though.

[Francesco Casella]

Replying to perost:

To me there are two kinds of structural parameters, actually structural parameters and parameters marked as structural because we want to evaluate them even though it's not strictly necessary.

Which I guess means they have annotation(Evaluate = true) and/or a final prefix, right?

Actually structural parameters must be possible to evaluate, so if they depend on non-fixed parameters we should probably give an error even if the non-fixed parameters have bindings.

Absolutely.

But as you say we should be more careful about marking other parameters as structural though.

That's the idea. This is currently not a top priority, but it would be very nice for us to have, because -d=evaluateAllParameters speeds up our models considerably, but we use this pattern with fixed = false parameters a lot, so -d=evaluateAllParameters won't work there.

[Per Östlund]

I've implemented a fix in 7c7c4af, which also enables evaluating final parameters that fulfill the requirements.

It's kind of basic right now, it only handles fixed = false|true and treats fixed = exp as fixed = false. It also has a limit to how deep it will follow the dependencies, for performance and to avoid dependency loops. Currently this limit is just 4, since parameters that we want to evaluate usually doesn't have deep dependency trees in my experience, but this can be tweaked if needed (in hindsight I should maybe have set it higher when -d=evaluateAllParameters is used).

We'll see what the damage is once Hudson has tested it, if needs be I can easily disable only the final parts and keep the -d=evaluateAllParameters fixes.

[Martin Sjölund]

-d=evaluateAllParameters is used in for example code generation for embedded systems where there can be no tuning of parameters and we really do want to evaluate everything to improve performance (even things with Evaluate=false).

[Per Östlund]

Replying to sjoelund.se:

-d=evaluateAllParameters is used in for example code generation for embedded systems where there can be no tuning of parameters and we really do want to evaluate everything to improve performance (even things with Evaluate=false).

Evaluate=false is always ignored by the NF, so nothing has changed there.

[Francesco Casella]

@sjoelund.se, you can't evaluate a parameter in the frontend if it doesn't have a binding equation, that's the point of this ticket.

If people write models with fixed = false parameters and initial equations to determine them (which are potentially coupled to regular equations, btw) and the embedded code generation can't handle them, too bad, but I don't see other solutions to the problem.

[Martin Sjölund]

For the frontend, yes. The backend has additional logic to evaluate parameters (I think also without binding equations in some cases).

[Francesco Casella]

Replying to perost:

I've implemented a fix in 7c7c4af, which also enables evaluating final parameters that fulfill the requirements.

Sounds good, the test now runs correctly when setting -d=evaluateAllParameters