Issue with records in HelmholtzMedia

[casella]

​HelmholtzMedia_HelmholtzMedia.Examples.BranchingDynamicPipes fails with

HelmholtzMedia_HelmholtzMedia.Examples.BranchingDynamicPipes_08bnd.c:4556:3: error: use of undeclared identifier 'pipe4_flowModel_Medium_ThermodynamicState'
  pipe4_flowModel_Medium_ThermodynamicState tmp5;
  ^

@mahge930, would you mind having a look what is wrong there?

[mahge930]

After a quick investigation, I think the correct type that should have been used here is

HelmholtzMedia_Examples_BranchingDynamicPipes_pipe4_flowModel_Medium_ThermodynamicState

I am trying to figure out what is happening that makes it lose the prefix and pick up a wrong type.

[casella]

Thanks @mahge930!

@matthis, we are getting there :)

[mahge930]

Here is some more context on what is happening here. I have tried to filter out the relevant parts.

• HelmholtzMedia.Examples.BranchingDynamicPipes extends Modelica.Fluid.Examples.BranchingDynamicPipes

• Modelica.Fluid.Examples.BranchingDynamicPipes has an element Pipes.DynamicPipe pipe4;

• Pipes.DynamicPipe extends BaseClasses.PartialTwoPortFlow

• BaseClasses.PartialTwoPortFlow has an element FlowModel flowModel;

where FlowModel = Modelica.Fluid.Pipes.BaseClasses.FlowModels.DetailedPipeFlow

• FlowModels.DetailedPipeFlow extends FlowModels.PartialGenericPipeFlow

FlowModels.PartialGenericPipeFlow extends FlowModels.PartialStaggeredFlowModel

• FlowModels.PartialStaggeredFlowModel has the element

parameter SI.DynamicViscosity mu_nominal = Medium.dynamicViscosity(
                                                 Medium.setState_pTX(
                                                     Medium.p_default, Medium.T_default, Medium.X_default))

The problem is here.

The binding equation for this component is a call with an argument that is a call to a function that returns a record (setState_pTX returns the record ThermoDynamicState).

In the flat model this binding is transformed to

HelmholtzMedia.Examples.BranchingDynamicPipes.pipe4.flowModel.Medium.dynamicViscosity(pipe4.flowModel.Medium.ThermodynamicState(1, 293.15, 101325.0, 1.839344937860023, 446511.6483341125, 501599.1949456299, 2722.459758941956))

This, I assume, is replacing the internal call with the evaluated return value of that function, which is a record. The evaluated return value is a call to the record constructor pipe4.flowModel.Medium.ThermodynamicState here. I am not sure where the constants used here are coming from, i.e., I am not sure which of the many replaceable setState_pTX and ThermoDunamicState elements this evaluation is using to achieve this.

At least, this call to the record constructor should have been prefixed properly to HelmholtzMedia.Examples.BranchingDynamicPipes.pipe4.flowModel.Medium.

[mahge930]

I can not help but feel that this call to record evaluation, even when prefixed correctly, might be picking up the wrong function anyway.

If you follow the extends chain above you can see that the call Medium.setState_pTX in the binding on mu_nominal should be using the version of setState_pTX from HelmholtzMedia.HelmHoltzFluids.Carbondioxide (a.k.a. HelmholtzMedia.Interfaces.PartialHelmholtzMedium). The latter is a much more involved and complicated function than the original setState_pTX in Modelica.Fluids. I would not expect it to be evaluated and inlined so easily.

@casella Maybe you can make more sense of it.

[perost]

I've fixed the prefixing issue in ​PR 6742, it turned out to be a trivial fix.

I also verified that the NF does use setState_pTX from HelmholtzMedia.HelmHoltzFluids.Carbondioxide. It is indeed quite a large function, but the NF intrepidly evaluates it anyway (it doesn't actually check the size of a function before trying to evaluate it, so ignorance is bliss I guess).

[perost]

Fixed in a2d2f913/OpenModelica. The model now compiles and simulates successfully, so I'm assuming this ticket is now resolved.

[matthis.thorade@…]

Great!

[casella]

Replying to perost:

I also verified that the NF does use setState_pTX from HelmholtzMedia.HelmHoltzFluids.Carbondioxide. It is indeed quite a large function, but the NF intrepidly evaluates it anyway (it doesn't actually check the size of a function before trying to evaluate it, so ignorance is bliss I guess).

I'm quite impressed. Evaluating HelmholtzMedia functions is no small feat :)

[matthis.thorade@…]

Same same, thanks for the effort put into this.
Hope the improvements also help elsewhere.

[perost]

Replying to matthis.thorade@…:

Hope the improvements also help elsewhere.

The ​results are in, it fixed BranchingDynamicPipes and nothing else (the other models are just unstable and show up in pretty much every report).

The issue only occurred when there was a function call with constant arguments that returned a record defined in a package in the root model and used in such a way that it couldn't be simplified away. The last part is only fulfilled in this case because the NF for some reason fails to evaluate the dynamicViscosity call that the result from setState_pTX is used as an argument to. That doesn't really matter since the backend can handle it, but I should probably check why it's failing to evaluate it.

[mahge930]

I also verified that the NF does use setState_pTX from HelmholtzMedia.HelmHoltzFluids.Carbondioxide. It is indeed quite a large function, but the NF intrepidly evaluates it anyway (it doesn't actually check the size of a function before trying to evaluate it, so ignorance is bliss I guess).

I had been conditioned by the OF that I thought this was not to be expected. I guess I underestimated the NF. I am sorry NF. :)

[perost]

The reason why the NF failed to evaluate dynamicViscosity turned out to be an error in the model. eta_r is used uninitialized on ​this line since it's an output with no default binding, which causes the evaluation to fail.

I tried to add an error message for trying to evaluate uninitialized variables, but it turned out to be more complicated than I thought because records are allowed to contain uninitialized variables as long as those aren't actually used. We currently hide any error messages when evaluating functions during the simplification step since such errors are usually not critical, unless -d=failtrace is used. A better solution would be to implement a static checker for uninitialized variables like the specification suggests.

[sjoelund.se]

Couldn't you evaluate it to an uninitialized value in that case?

[perost]

Replying to sjoelund.se:

Couldn't you evaluate it to an uninitialized value in that case?

You mean eta_r? Or the record issue?

[casella]

Replying to perost:

The reason why the NF failed to evaluate dynamicViscosity turned out to be an error in the model. eta_r is used uninitialized on ​this line since it's an output with no default binding, which causes the evaluation to fail.

I tried to add an error message for trying to evaluate uninitialized variables, but it turned out to be more complicated than I thought because records are allowed to contain uninitialized variables as long as those aren't actually used. We currently hide any error messages when evaluating functions during the simplification step since such errors are usually not critical, unless -d=failtrace is used. A better solution would be to implement a static checker for uninitialized variables like the specification suggests.

In any case, I'd suggest to fix the model by first declaring eta_r :=0, see ​#38.