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 )
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 , 4 years ago
Cc: | added |
---|
comment:2 by , 4 years ago
Summary: | NF cannot evaluate structural parameter → NF cannot evaluate structural parameter during NFTyping.typeComponents |
---|
comment:3 by , 4 years ago
Milestone: | 1.17.0 → 1.18.0 |
---|
comment:5 by , 4 years ago
Description: | modified (diff) |
---|
follow-up: 7 comment:6 by , 4 years ago
Description: | modified (diff) |
---|
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
follow-ups: 9 10 comment:7 by , 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 , 4 years ago
Cc: | added |
---|
comment:9 by , 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.
comment:10 by , 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:12 by , 4 years ago
Buildings.ThermalZones.Detailed.BaseClasses.Examples.InfraredRadiationExchange now runs fine, other models still have the "can't evaluate parameters" issue.
follow-up: 16 comment:13 by , 3 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
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.
follow-up: 15 comment:14 by , 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?
comment:15 by , 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.
comment:16 by , 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:18 by , 3 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
@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 , 3 years ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
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.
Andrea.Bartolini provided me a MWE to reproduce the same error (also attached)
When you try to compile
M2
, you getI 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.