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)
Change History (16)
by , 10 years ago
Attachment: | TestST3LRh_bypassTotal.zip added |
---|
comment:1 by , 10 years ago
Status: | new → accepted |
---|
comment:3 by , 10 years ago
Cc: | 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 be1000
andsteamTurbines.byPassLP.dpnom
should be5.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 forValveLiq1
, for which Av is computed correctly by the initial equation and the start attribute is ignored.
comment:4 by , 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.
follow-up: 6 comment:5 by , 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.
comment:6 by , 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.
follow-up: 8 comment:7 by , 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?
follow-up: 9 comment:8 by , 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.
follow-up: 10 comment:9 by , 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.
comment:10 by , 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:12 by , 10 years ago
Resolution: | → fixed |
---|---|
Status: | accepted → closed |
Save Total file