Opened 10 years ago

Closed 10 years ago

#3104 closed defect (fixed)

Incorrect handling of parameters with expression-determined fixed attribute

Reported by: Francesco Casella Owned by: Lennart Ochel
Priority: critical Milestone: 1.9.2
Component: Backend Version: trunk
Keywords: Cc: andrea.bartolini@…, Adrian Pop

Description

Please run the attached test script on the attached Save Total file, which reproduces ThermoPower.PowerPlants.SteamTurbineGroup.Tests.TestST3LRh_bypass.

There are two equations for steamTurbines.byPassLP.Av: initial equation no. 81

steamTurbines.byPassLP.Av=2.4027e-005 * steamTurbines.byPassLP.Cv

which is correct, and then later parameter equation 387

steamTurbines.byPassLP.Av=steamTurbines.byPassLP.wnom * DIVISION(ThermoPower.Water.ValveVap$steamTurbines$byPassLP.FlowChar(steamTurbines.byPassLP.thetanom), sqrt(steamTurbines.byPassLP.rhonom * steamTurbines.byPassLP.dpnom))

which is wrong, overrides the previous value and causes the simulation to crash.

The parameter in question is defined in ThermoPower.Water.BaseClasses.ValveBase as

    parameter SI.Area Av(
    fixed=if CvData == ThermoPower.Choices.Valve.CvTypes.Av then true else false,
    start=wnom/(sqrt(rhonom*dpnom))*FlowChar(thetanom))

The parameter steamTurbines.byPassLP.CvData is set to CvTypes.Cv, so it should have fixed = false and the initial equation

    Av = 2.4027e-5*Cv;

should apply.

For some reason, an additional parameter equation that sets the parameter equal to its start value and overrides the correct initial equation is added to the code. This shouldn't happen at all.

Attachments (4)

TestST3LRh_bypassTotal.zip (116.2 KB ) - added by Francesco Casella 10 years ago.
Save Total file
test.mos (229 bytes ) - added by Francesco Casella 10 years ago.
Test script
TestST3LRh_bypassTotal-update1.zip (116.3 KB ) - added by Francesco Casella 10 years ago.
Updated Save Total
test-update1.mos (247 bytes ) - added by Francesco Casella 10 years ago.
Updated test script

Download all attachments as: .zip

Change History (16)

by Francesco Casella, 10 years ago

Attachment: TestST3LRh_bypassTotal.zip added

Save Total file

by Francesco Casella, 10 years ago

Attachment: test.mos added

Test script

comment:1 by Lennart Ochel, 10 years ago

Status: newaccepted

comment:2 by Lennart Ochel, 10 years ago

This may be connected to #3108.

comment:3 by Francesco Casella, 10 years ago

Cc: Adrian Pop added

I did some more analysis, and in fact there are several separate problems with the attached TestST3LRh_bypassTotal:

  • the runtime complains about division by zero, but in fact steamTurbines.byPassLP.rhonom should be 1000 and steamTurbines.byPassLP.dpnom should be 5.94602e5. It seems that the backend is somehow losing their binding equations along the road
  • the runtime error is triggered by the evaluation of a start attribute which is actually not needed, since steamTurbines.byPassLP.Av is directly computed by an initial equation that gives its value explicitly
  • it is not clear to me from the dumpSimCode whether this start attribute would have actually been used. ThermoPower.Test.WaterComponents.TestCoeffValve has a similar setting for ValveLiq1, for which Av is computed correctly by the initial equation and the start attribute is ignored.

by Francesco Casella, 10 years ago

Updated Save Total

by Francesco Casella, 10 years ago

Attachment: test-update1.mos added

Updated test script

comment:4 by Francesco Casella, 10 years ago

I have updated the code, adding +1e-6 to rhonom and dpnom in order to avoid the division by zero and let the solver continue. I also put the condition for the fixed attribute in a separate final parameter with Evaluate = true, so the code now reads:

  final parameter Boolean fixedAv=
    (if CvData == ThermoPower.Choices.Valve.CvTypes.Av then true else false)
    annotation(Evaluate = true);
  parameter SI.Area Av(
    fixed=fixedAv,
    start=wnom/(sqrt(rhonom*dpnom))*FlowChar(thetanom)) 
    "Av (metric) flow coefficient";

Unfortunately, at this point the nonlinear solver fails because the start attribute of steamTurbines.ST_LP.hin, which is set to 1e5 by the Medium model, is lost somewhere in the process, and the value of zero is out of range of the IF97 medium model.

This issue is arleady reported in #3051 and #3095. I attached the updated save total for your convenience.

comment:5 by Volker Waurich, 10 years ago

Hi,
[24260] is sufficient to avoid the division by zero. Library changes are not needed anymore. The start values of the redeclared parameters are now used to initialize rhonom and dpnom.

The problem with the missing start value for steamTurbines.ST_LP.hin remains.

Last edited 10 years ago by Volker Waurich (previous) (diff)

in reply to:  5 comment:6 by Francesco Casella, 10 years ago

Replying to vwaurich:

[24260] is sufficient to avoid the division by zero. Library changes are not needed anymore. The start values of the redeclared parameters are now used to initialize rhonom and dpnom.

I'm not sure this really solves the problem.

In this specific example, there is a binding equation for rhonom in the valve model, and a binding equation for dpnom which comes from the HRSG model, which are somehow lost in the translation process. This should be fixed. Start values shouldn't be used as a safety net for compiler bugs :)

At any rate, please make sure that if a start attribute is used to determine a parameter, a warning is issued, as per the Modelica Specification (maybe this is already implemented, but I have to wait for the nightly build to test this myself). Otherwise we risk masking the problem in other cases, and potentially trust wrong simulation results.

comment:7 by Volker Waurich, 10 years ago

The binding equations are there. But only _dpnom has an initial value. .dpnom is initialized to 0. Since the division is evaluated before the binding equation, it fails.

My idea was to assign initial values of parameters from their binding if they would have had no initial value otherwise.

Another suggestion would be an ordering of binding parameter equations.
Would this be a proper fix?

in reply to:  7 ; comment:8 by Francesco Casella, 10 years ago

Replying to vwaurich:

The binding equations are there. But only _dpnom has an initial value. .dpnom is initialized to 0. Since the division is evaluated before the binding equation, it fails.

This is weird. The start attribute of Av is determined by an expression containing rhonom and dpnom. I would expect it gets evaluated after rhonom and dpnom have been evaluated, according to the BLT ordering.

My idea was to assign initial values of parameters from their binding if they would have
had no initial value otherwise.

This can be useful in case iterations are required to evaluate the parameter (when it has fixed = false and initial equations are involved), but it is no subsitute of proper BLT ordering.

Another suggestion would be an ordering of binding parameter equations.
Would this be a proper fix?

Absolutely! I thought we already had that. Maybe the problem is that expressions showing up in start attributes are not ordered together with the other ones, as I would have expected. This causes the evaluation of the start attribute for Av before dpnom and rhonom are evaluated according to their binding equations.

in reply to:  8 ; comment:9 by Volker Waurich, 10 years ago

Replying to casella:

Replying to vwaurich:

The binding equations are there. But only _dpnom has an initial value. .dpnom is initialized to 0. Since the division is evaluated before the binding equation, it fails.

This is weird. The start attribute of Av is determined by an expression containing rhonom and dpnom. I would expect it gets evaluated after rhonom and dpnom have been evaluated, according to the BLT ordering.

My idea was to assign initial values of parameters from their binding if they would have
had no initial value otherwise.

This can be useful in case iterations are required to evaluate the parameter (when it has fixed = false and initial equations are involved), but it is no subsitute of proper BLT ordering.

Another suggestion would be an ordering of binding parameter equations.
Would this be a proper fix?

Absolutely! I thought we already had that. Maybe the problem is that expressions showing up in start attributes are not ordered together with the other ones, as I would have expected. This causes the evaluation of the start attribute for Av before dpnom and rhonom are evaluated according to their binding equations.

This is already implemented but seems to be buggy in this case. I will check this.

in reply to:  9 comment:10 by Francesco Casella, 10 years ago

Replying to vwaurich:

Absolutely! I thought we already had that. Maybe the problem is that expressions showing up in start attributes are not ordered together with the other ones, as I would have expected. This causes the evaluation of the start attribute for Av before dpnom and rhonom are evaluated according to their binding equations.

This is already implemented but seems to be buggy in this case. I will check this.

OK, so we've eventually found the root cause of the bug :)

comment:11 by Volker Waurich, 10 years ago

Fixed in [24440]

comment:12 by Volker Waurich, 10 years ago

Resolution: fixed
Status: acceptedclosed
Note: See TracTickets for help on using tickets.