Opened 5 years ago

Closed 5 years ago

#5721 closed defect (fixed)

Redeclare extends on functions not picking up the correct redeclared package/record

Reported by: Mahder Alemseged Gebremedhin Owned by: Per Östlund
Priority: high Milestone: Future
Component: New Instantiation Version:
Keywords: Cc:

Description (last modified by Mahder Alemseged Gebremedhin)

If you flatten this model

model ConstantEffectiveness
 package Medium1 = BuildSysPro.IBPSA.Media.Water;
 package Medium2 = BuildSysPro.IBPSA.Media.Air;

  BuildSysPro.IBPSA.Fluid.Sources.Boundary_pT sou_2(
    redeclare package Medium = Medium2);

  BuildSysPro.IBPSA.Fluid.Sources.Boundary_pT sou_1(
    redeclare package Medium = Medium1);
end ConstantEffectiveness;

You get some functions from the components. e.g.

function ConstantEffectiveness.sou_1.Medium.specificEnthalpy "Return specific enthalpy"
  input ConstantEffectiveness.Medium.ThermodynamicState state "Thermodynamic state record";
  output Real h(quantity = "SpecificEnergy", unit = "J/kg", min = -10000000000.0, max = 10000000000.0, nominal = 1000000.0) "Specific enthalpy";
algorithm
  h := 4184.0 * (state.T - 273.15);
end ConstantEffectiveness.sou_1.Medium.specificEnthalpy;

this function is generated for sou_1 as you can see from the name. However, it seems the record type that is the input to the function is of type ConstantEffectiveness.Medium.ThermodynamicState. This type does not, strictly speaking, exist in the model since the partial Medium from which this type is picked up has been redeclared. It should have been ConstantEffectiveness.sou_1.Medium.ThermodynamicState from sou_1.

I am still not entirely sure what causes this. If you use just one of the components, sou_1 or sou_2 but not both then everything works fine. This is because even though we pick the wrong name/type for the record we at least pick the one with correct elements in it. However, if you use both then the name is used for both and we have issues at codegen time.

Maybe you can find something.

I am trying to duplicate it using a non-library simple model but so far doesn't seem to pinpoint it.

If you need the full model to test with, you can use BuildSysPro.IBPSA.Fluid.HeatExchangers.Validation.ConstantEffectiveness or one of the models from teh coverage where you see errors saying too many or too few arguments to function call ...

Change History (8)

comment:1 by Mahder Alemseged Gebremedhin, 5 years ago

Description: modified (diff)

comment:2 by Mahder Alemseged Gebremedhin, 5 years ago

Description: modified (diff)

in reply to:  description ; comment:3 by Per Östlund, 5 years ago

Replying to mahge930:

It should have been ConstantEffectiveness.sou_1.Medium.specificEnthalpy from sou_1.

You mean ConstantEffectiveness.sou_1.Medium.ThermodynamicState, right?

in reply to:  3 comment:4 by Mahder Alemseged Gebremedhin, 5 years ago

Description: modified (diff)

Replying to perost:

Replying to mahge930:

It should have been ConstantEffectiveness.sou_1.Medium.specificEnthalpy from sou_1.

You mean ConstantEffectiveness.sou_1.Medium.ThermodynamicState, right?

Oh yes. Updated.

comment:5 by Francesco Casella, 5 years ago

Possibly related to #5078, #5229, #5239, or #5279?

Those have been fixed in the meantime, though. Maybe there is some commonly recurring pattern that can be useful for troubleshooting this specific issue.

comment:6 by Mahder Alemseged Gebremedhin, 5 years ago

I think it is related more to #5075 and #5239.

#5075 was closed as duplicate of #5078. It probably was not the same issue.

In case you have not already started on this, the issue might be in how the function NFInstNode.scopePath deals with extends base classes and derived classes. It either does not have the full information, i.e., the component node (sou_1 in this cases), or it has the info and is not using it properly.

Version 0, edited 5 years ago by Mahder Alemseged Gebremedhin (next)

comment:7 by Per Östlund, 5 years ago

Here's a minimalish model that shows the issue:

partial package PartialSimpleMedium  
  record ThermodynamicState  
    Real p;
    Real T;
  end ThermodynamicState;

  function setState_pTX  
    input Real p;
    input Real T;
    output ThermodynamicState state;
  algorithm
    state := ThermodynamicState(p = p, T = T);
  end setState_pTX;

  replaceable partial function specificEnthalpy  
    input ThermodynamicState state;
    output Real h;
  end specificEnthalpy;
end PartialSimpleMedium;

model PartialSource_T  
  replaceable package Medium = PartialSimpleMedium;
  Real p_in_internal;
  Real T_in_internal;
  Real h_internal = Medium.specificEnthalpy(Medium.setState_pTX(p_in_internal, T_in_internal));
end PartialSource_T;

model ConstantEffectiveness  
  PartialSource_T sou_1(redeclare package Medium = PartialSimpleMedium);
end ConstantEffectiveness;

One issue is that the parent of any classes in an instance is set to be the class node and not the instance node (which is sometimes the same, sometimes not). That's a one line fix, though it seems to cause some issues, particularly with operator records. I need to investigate that a bit more.

What seems to be the big issue here is the redeclare of Medium though. The redeclared Medium node needs to be instantiated in the scope of ConstantEffectiveness where the modifier is declared, but at the same time its parent should be sou_1. Class nodes currently only have a parentScope node reference since the parent and scope is the same for normal classes, but this is not the case for redeclared classes.

I think splitting the parentScope into separate parent and scope references will not only solve this issue but will simplify the code and help with other issues too, so I will try that and see how it works.

comment:8 by Per Östlund, 5 years ago

Resolution: fixed
Status: newclosed

Fixed since 6a5601bf.

Note: See TracTickets for help on using tickets.