Opened 5 years ago

Closed 5 years ago

#5928 closed defect (fixed)

The backend should not silently discard initial consistent conditions or ignore initial inconsistent conditions

Reported by: Francesco Casella Owned by: Karim Adbdelhak
Priority: critical Milestone: 1.16.0
Component: Backend Version:
Keywords: Cc: Andrea Bartolini

Description

Please consider the following MWE, representing two point-mass objects which are rigidly connected to each other

model Consistent
  Real x1(start = 0, fixed = true);
  Real v1(start = 0, fixed = true);
  Real x2(start = 0, fixed = false);
  Real v2(start = 0, fixed = true);
  Real F;
equation
  der(x1) = v1;
  der(v1) = F;
  der(x2) = v2;
  der(v2) = -F+sin(time);
  x1 = x2;
end Consistent;

This model has two states after index reduction, and three fixed = true conditions, which are redundant but consistent. I would expect to get a warning about that, but I don't.

The following variant

model Inconsistent
  extends Consistent(v2(start = 1));
end Inconsistent;

has three redundant and inconsistent initial conditions, but is accepted and simulated, with the warning

[1] 11:41:30 Symbolic Warning
The model contains alias variables with conflicting start and/or 
nominal values. It is recommended to resolve the conflicts, because 
otherwise the system could be hard to solve. To print the conflicting
alias sets and the chosen candidates please use -d=aliasConflicts.

This warning would be appropriate in case there were no fixed = true modifiers and the tool had to pick between two potential states with different start value. But that is not the case here.

I guess this happens because index reduction here is handled by alias elimination. If that is the case, fixing this issue should be very easy

  1. if the start value of an alias variable with fixed = true is the same as the selected representative variable, issue a warning that a redundant consistent initial equation has been eliminated
  2. if the start value of an alias variable with fixed = true is not the same as the selected representative variable, issue an error that the model has inconsistent initial conditions:

Attachments (2)

TestFixed.mo​ (438 bytes ) - added by Francesco Casella 5 years ago.
TestFixed_2.mo​ (538 bytes ) - added by Karim Adbdelhak 5 years ago.

Download all attachments as: .zip

Change History (23)

by Francesco Casella, 5 years ago

Attachment: TestFixed.mo​ added

comment:1 by Francesco Casella, 5 years ago

Status: new → assigned

comment:2 by Karim Adbdelhak, 5 years ago

Regarding Point 2:
Do you expect the compilation process to fail entirely or just output an Error Message with "trying to continue with start value of <variablename>."?

Last edited 5 years ago by Karim Adbdelhak (previous) (diff)

comment:3 by Karim Adbdelhak, 5 years ago

Just as a reference, i think having it fail entirely could be a lot of work. The warning about inconsistent initial values is issued by many tests in our testsuite, we would have to adapt all of them accordingly (which i guess would be correct, but still a lot of work). It seems like even the MSL has models with inconsistent initial values.

The mentioned tests: 

flattening/libraries/3rdParty/siemens/translatePipes.mos
openmodelica/cppruntime/fmu/modelExchange/2.0/testDrumBoiler.mos
openmodelica/cppruntime/libraries/msl32/Modelica.Media.Examples.TestOnly.IdealGasN2.mos
openmodelica/cppruntime/libraries/msl32/Modelica.Media.Examples.Tests.MediaTestModels.Air.DryAirNasa.mos
openmodelica/cppruntime/libraries/msl32/Modelica.Media.Examples.WaterIF97.mos
openmodelica/cruntime/optimization/benchmark/runDrumBoiler.mos
openmodelica/cruntime/optimization/benchmark/runExReduceDrumBoiler.mos
openmodelica/cruntime/optimization/benchmark/runReduceDrumBoiler.mos
openmodelica/fmi/ModelExchange/2.0/testBug3763.mos
openmodelica/linearization/testDrumBoiler.mos
openmodelica/uncertainties/DataReconciliationTests21jan2013.mos
simulation/libraries/3rdParty/HumMod/buildHumModOMC.mos
simulation/libraries/3rdParty/TestMediaFrancesco/TestMedia.TestModels.WaterIF97.Test5.mos
simulation/libraries/3rdParty/TestMediaFrancesco/TestMedia.TestModels.WaterIF97.Test6.mos
simulation/libraries/3rdParty/TestMediaFrancesco/TestMedia.TestModels.WaterIF97.Test7.mos
simulation/libraries/3rdParty/TestMediaFrancesco/TestMedia.TestModels.WaterIF97.Test8.mos
simulation/libraries/3rdParty/ThermoPower/Bug2537.mos
simulation/libraries/3rdParty/ThermoPower/ThermoPower.Examples.HRB.Simulators.ClosedLoopDigitalSimulator.mos
simulation/libraries/3rdParty/ThermoPower/ThermoPower.Test.DistributedParameterComponents.TestFlow1D2phChen.mos
simulation/libraries/3rdParty/ThermoPower/ThermoPower.Test.DistributedParameterComponents.TestFlow1D2phDB_hf.mos
simulation/libraries/3rdParty/ThermoPower/ThermoPower.Test.DistributedParameterComponents.TestWaterFlow1DFV2ph.mos
simulation/libraries/3rdParty/ThermoPower/ThermoPower.Test.DistributedParameterComponents.TestWaterFlow1DFV_B.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.CombinedCyclePowerPlant.CombinedCycle_Load_100_50.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.CombinedCyclePowerPlant.CombinedCycle_TripTAC.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestBend.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestCentrifugalPump.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestCentrifugalPump4.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestCentrifugalPump7.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestCheckValve.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestCompressor.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestControlValve.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestDiaphragm.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestDynamicCentrifugalPump.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestDynamicCentrifugalPump1.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestDynamicCentrifugalPump2.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestDynamicCheckValve.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestDynamicDrum.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestDynamicOnePhaseFlowPipe.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestDynamicReliefValve.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestDynamicTwoPhaseFlowPipe.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestDynamicWaterHeating.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestDynamicWaterWaterExchanger.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestFan.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestFlueGasesJunctions.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestFlueGasesVolumes.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestFlueGasesVolumes1.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestIdealCheckValve.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestIdealSwitchValve.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestInvSingularPressureLoss.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestJunctions.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestJunctions2.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestJunctions3.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestJunctions4.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestJunctions5.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestLumpedStraightPipe.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestMassFlowMultiplier.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestNTUWaterHeating0.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestNTUWaterHeating1.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestNTUWaterHeating2.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestPipePressureLoss.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestRefP.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestSensors.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestSimpleEvaporatorWaterSteamFlueGases.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestSimpleStaticCondenser.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestSingularPressureLoss.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestStaticCentrifugalPump.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestStaticCentrifugalPump2.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestStaticCondenser.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestStaticDrum1.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestStaticDrum2.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestStaticExchangerWaterSteamFlueGases.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestStaticWaterWaterExchanger.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestStaticWaterWaterExchangerDTorWorEff.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestSteamDryer.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestSteamDryer2.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestSteamEngine.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestSteamExtractionSplitter.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestStodolaTurbine.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestStodolaTurbine1.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestStodolaTurbine2.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestStodolaTurbine3.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestSwitchValve.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestTank.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestThreeWayValve.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestVolumes.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestVolumes1.mos
simulation/libraries/3rdParty/ThermoSysPro/ThermoSysPro.Examples.SimpleExamples.TestVolumes2.mos
simulation/libraries/msl32/Modelica.Fluid.Examples.AST_BatchPlant.BatchPlant_StandardWater.mos
simulation/libraries/msl32/Modelica.Fluid.Examples.DrumBoiler.DrumBoiler.mos
simulation/libraries/msl32/Modelica.Fluid.Examples.Explanatory.MeasuringTemperature.mos
simulation/libraries/msl32/Modelica.Fluid.Examples.HeatExchanger.HeatExchangerSimulation-addDerAlias.mos
simulation/libraries/msl32/Modelica.Fluid.Examples.HeatExchanger.HeatExchangerSimulation.mos
simulation/libraries/msl32/Modelica.Fluid.Examples.HeatingSystem.mos
simulation/libraries/msl32/Modelica.Fluid.Examples.IncompressibleFluidNetwork.mos
simulation/libraries/msl32/Modelica.Fluid.Examples.InverseParameterization.mos
simulation/libraries/msl32/Modelica.Fluid.Examples.PumpingSystem.mos
simulation/libraries/msl32/Modelica.Fluid.Examples.TraceSubstances.RoomCO2.mos
simulation/libraries/msl32/Modelica.Fluid.Examples.TraceSubstances.RoomCO2WithControls.mos
simulation/libraries/msl32/Modelica.Mechanics.MultiBody.Examples.Constraints.PrismaticConstraint.mos
simulation/libraries/msl32/Modelica.Mechanics.MultiBody.Examples.Constraints.RevoluteConstraint.mos
simulation/libraries/msl32/Modelica.Mechanics.MultiBody.Examples.Constraints.SphericalConstraint.mos
simulation/libraries/msl32/Modelica.Mechanics.MultiBody.Examples.Constraints.UniversalConstraint.mos
simulation/libraries/msl32/Modelica.Media.Examples.MixtureGases.mos
simulation/libraries/msl32/Modelica.Media.Examples.MoistAir.mos
simulation/libraries/msl32/Modelica.Media.Examples.R134a.R134a1.mos
simulation/libraries/msl32/Modelica.Media.Examples.R134a.R134a2.mos
simulation/libraries/msl32/Modelica.Media.Examples.ReferenceAir.MoistAir.mos
simulation/libraries/msl32/Modelica.Media.Examples.TestOnly.FlueGas.mos
simulation/libraries/msl32/Modelica.Media.Examples.TestOnly.IdealGasN2.mos
simulation/libraries/msl32/Modelica.Media.Examples.TestOnly.IdealGasN2Mix.mos
simulation/libraries/msl32/Modelica.Media.Examples.TestOnly.MixIdealGasAir.mos
simulation/libraries/msl32/Modelica.Media.Examples.Tests.MediaTestModels.Air.DryAirNasa.mos
simulation/libraries/msl32/Modelica.Media.Examples.Tests.MediaTestModels.IdealGases.Air.mos
simulation/libraries/msl32/Modelica.Media.Examples.Tests.MediaTestModels.IdealGases.Nitrogen.mos
simulation/libraries/msl32/Modelica.Media.Examples.Tests.MediaTestModels.IdealGases.SimpleNaturalGas.mos
simulation/libraries/msl32/Modelica.Media.Examples.Tests.MediaTestModels.IdealGases.SimpleNaturalGasFixedComposition.mos
simulation/libraries/msl32/Modelica.Media.Examples.Tests.MediaTestModels.Water.IdealSteam.mos
simulation/libraries/msl32/Modelica.Media.Examples.Tests.MediaTestModels.Water.WaterIF97OnePhase_ph.mos
simulation/libraries/msl32/Modelica.Media.Examples.Tests.MediaTestModels.Water.WaterIF97_pT.mos
simulation/libraries/msl32/Modelica.Media.Examples.Tests.MediaTestModels.Water.WaterIF97_ph.mos
simulation/libraries/msl32/Modelica.Media.Examples.TwoPhaseWater.TestTwoPhaseStates.mos
simulation/libraries/msl32/Modelica.Media.Examples.WaterIF97.mos
simulation/modelica/daemode/testDAEmodeDrumBoiler.mos
simulation/modelica/functions_eval/MoistAir.mos
simulation/modelica/indexreduction/linearStateAlias_cse.mos
simulation/modelica/inheritances/Ticket4258a.mos
simulation/modelica/inheritances/Ticket4258b.mos
simulation/modelica/initialization/OverdeterminedInitialization.Fluid.DynamicPipeInitialValues.mos
simulation/modelica/initialization/OverdeterminedInitialization.Fluid.DynamicPipeLumpedPressureInitialization.mos
simulation/modelica/initialization/OverdeterminedInitialization.Fluid.DynamicPipesSeriesLargeNSteadyStateInitial.mos
simulation/modelica/initialization/OverdeterminedInitialization.Fluid.DynamicPipesSeriesSteadyStateInitial.mos
simulation/modelica/initialization/OverdeterminedInitialization.Fluid.TwoVolumesFullInitial.mos
simulation/modelica/initialization/OverdeterminedInitialization.Fluid.TwoVolumesFullInitialInconsistent.mos
simulation/modelica/initialization/OverdeterminedInitialization.Fluid.TwoVolumesFullSteadyStatePressureAndTemperature.mos
simulation/modelica/initialization/conflictingStartValues.mos
simulation/modelica/initialization/conflictingStartValues2.mos
simulation/modelica/jacobian/reuseConstantPartsJac1.mos
simulation/modelica/others/Bug3885.mos
simulation/modelica/others/EngineV6_evalParams.mos

comment:4 by Karim Adbdelhak, 5 years ago

Also: We do not have the comparison of two variables, rather a set of alias variables. A set of alias variables is considered fixed if any of the variables are fixed. I could adapt this such that the different fixed attributes still count for each start value.

NOTE: It is not important which variable actually gets chosen to remain and replace all others since it inherits the important attributes from the others. It is just the name that would be different (Might be important for dumps and debugging, but structurally it makes no difference).

I want to provide some examples and my thoughts on what happens with them, please correct me if you disagree!

1. Alias Set:
a - fixed   - start=1
b - unfixed - start=2
c - fixed   - start=1
-> WARNING: Redundant start values for either a or c have been detected during alias elimination.

2. Alias Set:
a - fixed   - start=1
b - unfixed - start=2
c - unfixed - start=1
-> NO WARNING

3. Alias Set:
a - unfixed - start=1
b - unfixed - start=2
c - unfixed - start=1
-> NO WARNING

4. Alias Set:
a - fixed   - start=1
b - fixed   - start=2
c - unfixed - start=1
-> ERROR: Conflicting start values have been detected for a and b during alias elimination.

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

Replying to Karim.Abdelhak:

Regarding Point 2:
Do you expect the compilation process to fail entirely

Yes. If I prescribe inconsistent initial equations with fixed = true attributes, the model is invalid.

In case there are two potential state variables with fixed = false and inconsistent start attributes, if I don't provide explicit initial conditions the tool is authorized to take some chance in picking the fixed = true variables, and notify me what it did. This is fine: I'm not providing enough info to uniquely identify the initial conditions, the tool takes some chances and tells me that.

On the other hand, if I do provide wrong explicit redundant and inconsistent initial conditions, the tool should report that and stop, not try to fix the model in my place. BTW, that's what also Dymola does. For the Inconsistent model, it reports

The initialization problem is overspecified for variables of element type Real.
The equation 
v2 = v1;
refers to variables, which all are known.
To correct it you can remove this equation or inactivate start values for 
v1(start = 0)
v2(start = 1)

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

Replying to Karim.Abdelhak:

Just as a reference, i think having it fail entirely could be a lot of work. The warning about inconsistent initial values

Inconsistent start attributes are in general ok, particularly until we fix the consistency criterion, which is currently bogus, see #5813.

What is not ok is having variables with inconsistent fixed = true start attributes!

in reply to:  4 ; comment:7 by Francesco Casella, 5 years ago

Replying to Karim.Abdelhak:

Also: We do not have the comparison of two variables, rather a set of alias variables. A set of alias variables is considered fixed if any of the variables are fixed.

Why?

I could adapt this such that the different fixed attributes still count for each start value.

Absolutely! ​Section 8.6 of the specification says:

Further constraints, necessary to determine the initial values of all variables, can be defined in the following ways: ... For all non-discrete (that is continuous-time) Real variables vc, the equation vc = startExpression is added to the initialization equations, if start = startExpression and fixed = true.

I don't think there is any margin of interpretation in case there are inconsistent initial equations stemming from this rule. You can't have x(0) = 1 and x(0) = 2 at the same time, and it you try to get that, it's most definitely an error.

BTW, in general these initial equations should be considered together with other equations in the initial equation sections. On the other hand, I guess the detection of consistent or inconsistent initial equations deriving from fixed = true attributes could be partially performed when handling alias sets already, without the need of the sophisticated analysis on the dependency graph that is later performed on possibly redundant initial equation sets.

What do you think?

in reply to:  7 ; comment:8 by Karim Adbdelhak, 5 years ago

Replying to casella:

Replying to Karim.Abdelhak:

Also: We do not have the comparison of two variables, rather a set of alias variables. A set of alias variables is considered fixed if any of the variables are fixed.

Why?

Well it is way more efficient. Please consider a system containing

 a = b;
 c = b;
 d = b;
 e = b;

If we handled each individually we would have to do the analysis four times and apply replacement rules four times. If we just collect the alias set {a,b,c,d,e} we know that we will only keep one of all of them and can make the correct decision without fearing we could revoke it anyways. And if any of those variables is fixed, the remaining variable replacing all others (whatever it will be) will be fixed.

But for the selection of the attributes i can still consider individual fixed attributes and report errors or warnings for special cases. I guess we should throw a warning whenever multiple variables of one set are fixed, because the user thinks he fixes enough variables, when in reality he is only fixing the same variable multiple times. Even if both have default start value right?

An error can be issued if they contain different start values.

I could adapt this such that the different fixed attributes still count for each start value.

Absolutely! ​Section 8.6 of the specification says:

Further constraints, necessary to determine the initial values of all variables, can be defined in the following ways: ... For all non-discrete (that is continuous-time) Real variables vc, the equation vc = startExpression is added to the initialization equations, if start = startExpression and fixed = true.

I don't think there is any margin of interpretation in case there are inconsistent initial equations stemming from this rule. You can't have x(0) = 1 and x(0) = 2 at the same time, and it you try to get that, it's most definitely an error.

BTW, in general these initial equations should be considered together with other equations in the initial equation sections. On the other hand, I guess the detection of consistent or inconsistent initial equations deriving from fixed = true attributes could be partially performed when handling alias sets already, without the need of the sophisticated analysis on the dependency graph that is later performed on possibly redundant initial equation sets.

What do you think?

Remaining start equations will later on be evaluated with all other initial equations while balancing the initial equations (something lennart wrote i think). This part of alias elimination is just a pre-check where obvious alias mistakes are filtered.

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

Replying to Karim.Abdelhak:

Well it is way more efficient. Please consider a system containing

 a = b;
 c = b;
 d = b;
 e = b;

If we handled each individually we would have to do the analysis four times and apply replacement rules four times. If we just collect the alias set {a,b,c,d,e} we know that we will only keep one of all of them and can make the correct decision without fearing we could revoke it anyways.

Agreed.

And if any of those variables is fixed, the remaining variable replacing all others (whatever it will be) will be fixed.

Yes, provided there is only one fixed variable in the set. If there are more then one, either they are fixed to the same value (-> warn of redundant consistent initial condition) or they are fixed to different values (-> error, inconsistent initial conditions).

But for the selection of the attributes i can still consider individual fixed attributes and report errors or warnings for special cases. I guess we should throw a warning whenever multiple variables of one set are fixed, because the user thinks he fixes enough variables, when in reality he is only fixing the same variable multiple times. Even if both have default start value right?

An error can be issued if they contain different start values.

Agreed.

Remaining start equations will later on be evaluated with all other initial equations while balancing the initial equations (something lennart wrote i think). This part of alias elimination is just a pre-check where obvious alias mistakes are filtered.

Sounds good.

by Karim Adbdelhak, 5 years ago

Attachment: TestFixed_2.mo​ added

comment:10 by Karim Adbdelhak, 5 years ago

I made some changes which need some work polishing and updating testsuite, so before i put in the work i would like some conformation that the following behaviour is what is expected:

I added a small extension to your examples (see attachment):

 model Consistent2
   extends Consistent(v2(start = 1, fixed=false));
 end Consistent2;

Consistent has redundant fixed start values and prints:

Warning: The model contains alias variables with conflicting nominal or redundant start values. It is recommended to resolve the conflicts, because otherwise the system could be hard to solve. To print the conflicting alias sets and the chosen candidates please use -d=aliasConflicts.

Consistent2 has conflicting start values, but only one is fixed. -> No output.

Inconsistent has conflicting start values, both are fixed and prints:

Error: The model contains alias variables with conflicting fixed start values. To print the conflicting alias sets and the chosen candidates please use -d=aliasConflicts.
Warning: It was not possible to determine if the initialization problem is consistent, because of not evaluable parameters/start values during compile time: v1 = $START.v1 ($START.v2 = $START.v1)
Warning: The initial conditions are over specified. For more information set -d=initialization. In OMEdit Tools->Options->Simulation->OMCFlags, in OMNotebook call setCommandLineOptions("-d=initialization").
Error: The model contains alias variables with conflicting fixed start values. To print the conflicting alias sets and the chosen candidates please use -d=aliasConflicts.
Error: post-optimization module removeSimpleEquations (simulation) failed.

And fails afterwards. It is a bit verbose right now, but i could clean that up a bit.

Is this the expected behaviour?

comment:11 by Francesco Casella, 5 years ago

Sounds good!

comment:12 by Francesco Casella, 5 years ago

@Karim, were your latest changes already incorporated in ​PR 829? If so, you may close the ticket.

Thanks!

in reply to:  12 ; comment:13 by Karim Adbdelhak, 5 years ago

Replying to casella:

@Karim, were your latest changes already incorporated in ​PR 829? If so, you may close the ticket.

Thanks!

No, unfortunately i accidentally deleted my branch containing the changes. I need to stop naming the branches after ticket ids.

I picked up the work again and recovered most of it. But i stumbled upon one last question:
We have this confidence number check, that selects a start value in case of conflicts. Is it really expected to hard crash on me if we have two alias states, one being x and one being model.some.thing.very.deep.x, both being fixed and having start values.

Right now it just takes the start value of x because it has a better confidence number due to cref depth and reports a warning.

Don't understand me wrong, we have had this conversation for many times. Structurally i would totaly throw an error because it is ambigous for initialization, but i don't know if that is the behaviour you expect.

in reply to:  13 ; comment:14 by Francesco Casella, 5 years ago

Replying to Karim.Abdelhak:

Replying to casella:

@Karim, were your latest changes already incorporated in ​PR 829? If so, you may close the ticket.

Thanks!

No, unfortunately i accidentally deleted my branch containing the changes. I need to stop naming the branches after ticket ids.

I guess you should also avoid deleting branches, until they are at least six months old, just in case :)

I picked up the work again and recovered most of it. But i stumbled upon one last question:
We have this confidence number check, that selects a start value in case of conflicts. Is it really expected to hard crash on me if we have two alias states, one being x and one being model.some.thing.very.deep.x, both being fixed and having start values.

Right now it just takes the start value of x because it has a better confidence number due to cref depth and reports a warning.

I'm afraid this is really not correct in general. If both are fixed, it is an error if the values are different, or a warning (one redundant consistent initial condition) if they have the same start attribute, no matter what the confidence number is.

Don't understand me wrong, we have had this conversation for many times. Structurally i would totaly throw an error because it is ambigous for initialization, but i don't know if that is the behaviour you expect.

It depends on the start values. y(fixed = true) means that the equation y = y.start is added to the initialization system. Here you'd have two such equations. If the two start attributes are equal, it's redundant but consistent (-> issue a warning, then proceed), otherwise it's redundant and inconsistent (-> issue an error and abort)

in reply to:  14 comment:15 by Karim Adbdelhak, 5 years ago

Replying to casella:

Replying to Karim.Abdelhak:

No, unfortunately i accidentally deleted my branch containing the changes. I need to stop naming the branches after ticket ids.

I guess you should also avoid deleting branches, until they are at least six months old, just in case :)

It gets messy really quick, maybe i just push every branch to my repo before deleting so i do not have to deal with them locally.

It depends on the start values. y(fixed = true) means that the equation y = y.start is added to the initialization system. Here you'd have two such equations. If the two start attributes are equal, it's redundant but consistent (-> issue a warning, then proceed), otherwise it's redundant and inconsistent (-> issue an error and abort)

Okay perfect, that is how i implemented it.

I had 218 failing tests first, but it turns out they are all due to my changes in the warning string, no actual errors. I will update the testsuite and push it, should be done tomorrow!

comment:16 by Francesco Casella, 5 years ago

Excellent!

comment:17 by Philip Hannebohm, 5 years ago

Replying to Karim.Abdelhak:

We have this confidence number check, that selects a start value in case of conflicts.

This confidence number could be valuable for selecting a nominal value in the case of redundant but consistent start values?
(I only mention them because they appear in one of the error messages, no idea what I'm talking about.)

in reply to:  17 comment:18 by Karim Adbdelhak, 5 years ago

Replying to phannebohm:

Replying to Karim.Abdelhak:

We have this confidence number check, that selects a start value in case of conflicts.

This confidence number could be valuable for selecting a nominal value in the case of redundant but consistent start values?
(I only mention them because they appear in one of the error messages, no idea what I'm talking about.)

Yes the confidence number will still be used for nominal and min/max attributes. I guess when implementing this they did not think of treating the start value differently.

in reply to:  17 comment:19 by Francesco Casella, 5 years ago

Replying to phannebohm:

Replying to Karim.Abdelhak:

We have this confidence number check, that selects a start value in case of conflicts.

This confidence number could be valuable for selecting a nominal value in the case of redundant but consistent start values?

Absolutely. We have the same problem there, that the current selection algorithm is not good at all, see #5813

comment:20 by Karim Adbdelhak, 5 years ago

This issue should be resolved with ​PR845.

It now reports redundant fixed values and throws an error for conflicting fixed values. I guess for the issue regarding unfixed values we will continue in #5813.

comment:21 by Karim Adbdelhak, 5 years ago

Resolution: → fixed
Status: assigned → closed
Note: See TracTickets for help on using tickets.