Opened 4 years ago

Closed 3 years ago

#5991 closed defect (fixed)

NF cannot evaluate structural parameter during NFTyping.typeComponents

Reported by: Francesco Casella Owned by: Per Östlund
Priority: blocker Milestone: 1.18.0
Component: New Instantiation Version:
Keywords: Cc: Andrea Bartolini, Michael Wetter

Description (last modified by Francesco Casella)

There are over 50 models in the Buildings library that fail for the same reason, see, e.g., Buildings.ThermalZones.Detailed.BaseClasses.Examples.InfraredRadiationExchange. The NF reports

[Buildings latest/HeatTransfer/Data/OpaqueConstructions.mo:9:4-11:111:writable]
Error: Could not evaluate structural parameter (or constant): 
irRadExc.datConExt.layers.nLay which gives dimensions of array: material.
Array dimensions must be known at compile time.

There are many other models with the same issue, e.g. Buildings.Examples.ScalableBenchmarks.BuildingVAV.Examples.OneFloor_OneZone. The NF fails with

Error: Internal error 
Instantiation of Buildings.Examples.ScalableBenchmarks.BuildingVAV.Examples.OneFloor_OneZone
failed with no error message.

This issue affects a large number of models in the Buildings libraries, e.g.
https://libraries.openmodelica.org/branches/master/Buildings_latest/files/Buildings_latest_Buildings.ThermalZones.Detailed.Constructions.Examples.ExteriorWall.err
https://libraries.openmodelica.org/branches/master/Buildings_latest/files/Buildings_latest_Buildings.ThermalZones.Detailed.Examples.ElectroChromicWindow.err
https://libraries.openmodelica.org/branches/master/Buildings_latest/files/Buildings_latest_Buildings.ThermalZones.Detailed.Examples.MixedAirCO2.err
Fixing this issue would improve the coverage of Buildings substantially.

Change History (19)

comment:1 by Francesco Casella, 4 years ago

Cc: Andrea Bartolini added

Andrea.Bartolini provided me a MWE to reproduce the same error (also attached)

package TestRecord
  record CalcData
    parameter Integer N = 1;
    Real[N] val;
  end CalcData;

  model M1
    input CalcData calcData;
    Real x[calcData.N];
  equation
    x = calcData.val.^2;
  end M1;

  model M2
    constant Integer Nact = 5;
    CalcData calcVal(N=Nact);
    M1 m1(calcData = calcVal);
  equation
    calcVal.val = fill(time,Nact);
  end M2;
end TestRecord;

When you try to compile M2, you get

[1] 23:01:21 Translation Error
[TestRecord: 4:5-4:16]: Could not evaluate structural parameter (or constant): 
m1.calcData.N which gives dimensions of array: val. 
Array dimensions must be known at compile time.

I suspect the issue is the same that plagues the Building library, which often uses record to hold datasheets, which may contain structural parameters for array dimensions.

As far as I understand this model is perfectly legal. Dymola accepts it also in pedantic mode.

comment:2 by Francesco Casella, 4 years ago

Summary: NF cannot evaluate structural parameterNF cannot evaluate structural parameter during NFTyping.typeComponents

comment:3 by Francesco Casella, 4 years ago

Milestone: 1.17.01.18.0

comment:4 by Francesco Casella, 4 years ago

See also #6126.

comment:5 by Francesco Casella, 4 years ago

Description: modified (diff)

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

Replying to casella:

Other models with the same issue after recent commits solving upstream problems
Buildings.Examples.ScalableBenchmarks.BuildingVAV.Examples.OneFloor_OneZone
Buildings.ThermalZones.Detailed.Constructions.Examples.ExteriorWall
Buildings.ThermalZones.Detailed.Examples.ElectroChromicWindow
Buildings.ThermalZones.Detailed.Examples.MixedAirCO2

After the latest commits, these models all fail because of

[/OMCompiler/Compiler/NFFrontEnd/NFExpression.mo:1407:7-1408:100:writable] Error:
Internal error NFExpression.makeSubscriptedExp: too few dimensions in 
Buildings.HeatTransfer.Data.OpaqueConstructions.Brick120(1, 
{Buildings.HeatTransfer.Data.Solids.Brick(0.12, 0.89, 790.0, 1920.0, 0.1348314606741573,
 3, 2, false, 331.4, 156.6572154293169, 1.418140151743967, 293.15, 293.15, 0.0, false, false)},
0.1348314606741573, {2}, 0.9, 0.9, 0.5, 0.5, Buildings.HeatTransfer.Types.SurfaceRoughness.Medium) to apply subscripts [<conExt, 1>]
[Buildings latest/HeatTransfer/Data/OpaqueConstructions.mo:9:4-11:111:writable] Error: 
Could not evaluate structural parameter (or constant): 
buiZon.theZon.roo.conExt.layers.nLay which gives dimensions of array: material.
Array dimensions must be known at compile time.

so there is another issue (too few dimensions) before getting to "can't evaluate parameter".

comment:8 by Francesco Casella, 4 years ago

Cc: Michael Wetter added

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

Replying to casella:

so there is another issue (too few dimensions) before getting to "can't evaluate parameter".

This issue seems to affect a large fraction of the models that still fail in the frontend.

in reply to:  7 comment:10 by Per Östlund, 4 years ago

Replying to casella:

so there is another issue (too few dimensions) before getting to "can't evaluate parameter".

The first error message is the reason why it fails to evaluate the parameter. It probably failed for a similar reason previously, it just failed silently instead of detecting that something had gone wrong. It's a stumble in the right direction at least.

comment:11 by Francesco Casella, 4 years ago

I hope that's an easy fix.

comment:12 by Francesco Casella, 4 years ago

Buildings.ThermalZones.Detailed.BaseClasses.Examples.InfraredRadiationExchange now runs fine, other models still have the "can't evaluate parameters" issue.

comment:13 by Per Östlund, 3 years ago

Resolution: fixed
Status: newclosed

Fixed in 49e1e9e/OpenModelica.

The regression report is unfortunately a bit messy due to a lot of libraries being updated at the same time. It seems only two models went from failing in the frontend to simulating and four models from failing during code gen to simulating. A bunch of ModelicaTest models started failing also, but I'm already pushing in a fix for that.

Most of the still failing models in Buildings did improve by my commit though, but a lot of them are now failing due to what seems to be type issues that while correctly identified should maybe be ignored due to conditional components that are removed.

comment:14 by Francesco Casella, 3 years ago

Well, six more models are already something, we're now at 87% success rate and counting.

I stand by for the fix that restores the ModelicaTest models. That is definitely needed before we branch off 1.18.0 :)

Most of the still failing models in Buildings did improve by my commit though, but a lot of them are now failing due to what seems to be type issues that while correctly identified should maybe be ignored due to conditional components that are removed.

Is that legal, according to the Modelica Specification?

in reply to:  14 comment:15 by Per Östlund, 3 years ago

Replying to casella:

Well, six more models are already something, we're now at 87% success rate and counting.

I stand by for the fix that restores the ModelicaTest models. That is definitely needed before we branch off 1.18.0 :)

Most of the still failing models in Buildings did improve by my commit though, but a lot of them are now failing due to what seems to be type issues that while correctly identified should maybe be ignored due to conditional components that are removed.

Is that legal, according to the Modelica Specification?

The specification says that conditional components where the condition is false is added and then removed from the flattened DAE, to ensure that the "component is valid". That was added in 3.5, but it still leaves a lot to interpretation since it's not specified when e.g. type checking is done.

In the NF we allow binding equations to have the wrong type as long as the component is removed, since many libraries expect that to work. It seems this is either not working correctly for the Buildings models or something else is going on.

in reply to:  13 comment:16 by Martin Sjölund, 3 years ago

Replying to perost:

Fixed in 49e1e9e/OpenModelica.

The regression report is unfortunately a bit messy due to a lot of libraries being updated at the same time.

Actually, I changed which machine is used to run the tests in order to not have to wait so long for the library coverage to finish. It seems to have also listed this as a configuration change, but that might just be some paths changing.

comment:17 by Francesco Casella, 3 years ago

The ModelicaTest stuff is now working again, see the last report

comment:18 by Francesco Casella, 3 years ago

Resolution: fixed
Status: closedreopened

@perost, if you don't mind I would keep this ticket open until the issue that you pointed out in comment:15 is also resolved. Until then, we still do have models in the library, e.g.
Buildings.Examples.ScalableBenchmarks.BuildingVAV.Examples.OneFloor_OneZone, that are failing because constants cannot be evaluated during NFTyping.typeComponents

I understand it's for a different reason than for other examples, such as Buildings.ThermalZones.Detailed.BaseClasses.Examples.InfraredRadiationExchange, who have now been fixed, and that you have already identified the root cause.

However, we still need to get this fixed to reach the 100% success rate goal of the LBL DFD, and I see no point opening another ticket with almost the same subject, this one, and specifically comment:15, should be good enough.

comment:19 by Francesco Casella, 3 years ago

Resolution: fixed
Status: reopenedclosed

PR #7608 has further improved the situation, now all remaining models failing in the frontend fail for some other reported issues - some of them still lead to "cannot evaluate structural parameter", but the root cause is identified by the previous message.

I think we can now close this ticket, and open new ones with the remaining issues. Currently 95% of the models of Buildings 7.0.1 pass the frontend phase, there are 63 remaining failing models, because of a handful of remaining issues, probably six or seven.

Note: See TracTickets for help on using tickets.