Opened 6 years ago

Closed 6 years ago

#5254 closed defect (fixed)

Issue with constaining class in replaceable array of models

Reported by: Francesco Casella Owned by: Per Östlund
Priority: high Milestone: 1.14.0
Component: New Instantiation Version:
Keywords: Cc:

Description (last modified by Adrian Pop)

Please check IBPSA.Fluid.MixingVolumes.BaseClasses.Validation.MixingVolumeHeatMoisturePort. The NF fails with the errors:

[IBPSA latest/Fluid/MixingVolumes/BaseClasses/Validation/MixingVolumeHeatPortWater.mo:15:10-15:67:writable] Warning: 'each' used when modifying non-array element vol.
[IBPSA latest/Fluid/MixingVolumes/BaseClasses/Validation/MixingVolumeHeatPortWater.mo:16:11-16:77:writable] Notification: From here:
[IBPSA latest/Fluid/MixingVolumes/BaseClasses/PartialMixingVolume.mo:6:3-8:71:writable] Error: Type mismatch in binding initialize_p = {i == 1 and not Medium.singleState for i in 1:3}, expected subtype of Boolean, got type Boolean[3].

The offending code is found here; I'm not sure if it is correct and OMC is wrong, or the other way round.

Change History (9)

comment:1 by Michael Wetter, 6 years ago

The error

Buildings_latest_Buildings.Applications.DataCenters.ChillerCooled.Equipment.Validation.ElectricChillerParallel.err:
[/var/lib/hudson/slave/workspace/OpenModelica_TEST_LIBS/OpenModelica/OMCompiler/build/lib/omlibrary/Buildings 
latest/Fluid/Chillers/ElectricEIR.mo:14:3-17:70:writable] Error: Type mismatch in binding per = chiPar.per, 
expected subtype of Generic, got type Generic[2].

seems to be related. I can't see anything wrong with this model.

comment:2 by Adrian Pop, 6 years ago

Hmmm, this seems to be a bit tricky as far as I can understand from the Modelica 3.4 spec (7.3.2 Constraining Type).

Examples:

replaceable T1 x[n] constrainedby T2; // your case, T1 should have the same dimensions as T2
replaceable type T=T1[n] constrainedby T2;
replaceable T1[n] x constrainedby T2;

In these examples the number of dimensions must be the same in T1 and T2, as well as in a redeclaration. Normally T1 and T2 are scalar types, but both could also be defined as array types – with the same number of dimensions. Thus if T2 is a scalar type (e.g. type T2= Real) then T1 must also be a scalar type; and if T2 is defined as vector type (e.g. type T2=Real[3]) then T1 must also be vector type.

As far as I can see from your declaration:

replaceable IBPSA.Fluid.MixingVolumes.BaseClasses.MixingVolumeHeatPort vol[nEle]
    constrainedby IBPSA.Fluid.MixingVolumes.BaseClasses.MixingVolumeHeatPort(
    redeclare each package Medium = Medium,
    each energyDynamics=Modelica.Fluid.Types.Dynamics.FixedInitial,
    final initialize_p={(i == 1 and not Medium.singleState) for i in 1:nEle},
    each m_flow_nominal=1,
    each V=1,
    each nPorts=2) "Mixing volume";

As far as I can interpret this, you should not use each in the constrainedby part as it refers to the type of component vol without the array dimension (which applies to the component, not its type).

comment:3 by Adrian Pop, 6 years ago

Description: modified (diff)

comment:4 by Adrian Pop, 6 years ago

If you look at this part alone:

IBPSA.Fluid.MixingVolumes.BaseClasses.MixingVolumeHeatPort(
    redeclare each package Medium = Medium,
    each energyDynamics=Modelica.Fluid.Types.Dynamics.FixedInitial,
    final initialize_p={(i == 1 and not Medium.singleState) for i in 1:nEle},
    each m_flow_nominal=1,
    each V=1,
    each nPorts=2) "Mixing volume";

the each and the array binding is wrong.

You should be able to say it like (using nEle array dimension):

IBPSA.Fluid.MixingVolumes.BaseClasses.MixingVolumeHeatPort[nEle](
    redeclare each package Medium = Medium,
    each energyDynamics=Modelica.Fluid.Types.Dynamics.FixedInitial,
    final initialize_p={(i == 1 and not Medium.singleState) for i in 1:nEle},
    each m_flow_nominal=1,
    each V=1,
    each nPorts=2) "Mixing volume";

but you cannot actually write this because the grammar doesn't allow it.

As far as I can tell the only proper way to write this is:

model T = IBPSA.Fluid.MixingVolumes.BaseClasses.MixingVolumeHeatPort[nEle];

replaceable T vol
    constrainedby T(
    redeclare each package Medium = Medium,
    each energyDynamics=Modelica.Fluid.Types.Dynamics.FixedInitial,
    final initialize_p={(i == 1 and not Medium.singleState) for i in 1:nEle},
    each m_flow_nominal=1,
    each V=1,
    each nPorts=2) "Mixing volume";

We'll wait for Per to clarify how he interpreted the specification.

comment:5 by Adrian Pop, 6 years ago

@casella, how do you open a ticket in the Modelica Specification Trac?
I cannot find anymore "New Ticket" link in there. Eh, I'll bother Martin about this.

We need to clarify this at the specification level as is really poorly specified.
The declaration in your code here only makes sense if the array dimension of the component are automatically applied to the constrainedby clause.

comment:6 by Adrian Pop, 6 years ago

The bug might also come from here:

model MixingVolumeHeatMoisturePort
  "Validation model for setting the initialization of the pressure for model with moisture port"
  extends IBPSA.Fluid.MixingVolumes.BaseClasses.Validation.MixingVolumeHeatPortWater(
    redeclare package Medium = IBPSA.Media.Air,
    redeclare each IBPSA.Fluid.MixingVolumes.BaseClasses.MixingVolumeHeatMoisturePort vol);
// ...
end MixingVolumeHeatMoisturePort;

If I remember correctly from the discussions in the Modelica Design Meetings on redeclare, the array dimension can be omitted and should be copied from the replaceable declaration. It seems that the NF does not do that and actually redeclares vol as a non-array. Truth is that the each in this case is wrong, as you modify the entire array (because the dimensions are copied), not each element.

in reply to:  6 comment:7 by Michael Wetter, 6 years ago

This is now fixed on the Buildings master, and there is a pull request to also fix it on the IBPSA master.

comment:8 by Per Östlund, 6 years ago

After the model was fixed upstream it had too few equations when using the new frontend, this has now been fixed in dc82eeb. There are still issues with array modifiers though, which causes the flat model to contain things like:

if {true, false, false} then
  vol[1].dynBal.medium.p = vol[1].dynBal.p_start;
end if;

I.e. the condition in the if-equation is evaluated incorrectly and uses the whole binding equation of e.g. vol.dynBal.initialize_p instead of vol[1].dynBal.initialize_p.

comment:9 by Francesco Casella, 6 years ago

Milestone: 2.0.01.14.0
Resolution: fixed
Status: newclosed

OK, I think we can close this ticket, as the original issue has been solved. I'll open a separate one on the last remaining issue.

Note: See TracTickets for help on using tickets.