OpenModelica icon indicating copy to clipboard operation
OpenModelica copied to clipboard

Issue with protected parts in records in Buildings

Open casella opened this issue 3 years ago • 6 comments

@perost, among the various issues in the latest regression report, there are Buildings models such as Buildings.Fluid.HeatPumps.BaseClasses.Validation.EquationFitReversible that fail during flattening with

[Buildings 9.0.0-master/Fluid/HeatPumps/Data/EquationFitReversible/Generic.mo:21:3-49:25:writable]
Error: Protected sections are not allowed in record.

The offending code is here, the protected clause was added 3 years ago.

@perost, what is wrong with that?

Keeping @mwetter in the loop.

casella avatar May 14 '22 11:05 casella

@perost, what is wrong with that?

4.6 states that records have the following restriction:

Only public sections are allowed in the definition or in any of its components (i.e., equation, algorithm, initial equation, initial algorithm and protected sections are not allowed)

This has been the case since Modelica 3.0, but I only just added that check to the new frontend. I'm not sure if we should keep the error message as it is or change it to just be a warning, but the library should be fixed regardless if it wants to be compliant.

I'm not sure why this restriction exists though, in Modelica 2.2 and earlier it only forbid protected components and not all protected sections. I don't really see why protected classes would be an issue, so it might be possible to get the specification changed if Buildings has a good use case for it.

perost avatar May 14 '22 13:05 perost

I guess one problem with that pattern is that you cannot create temporary variables of the protected class, for example in a function. Not sure why it would be forbidden, but it seems it is.

sjoelund avatar May 14 '22 13:05 sjoelund

@mwetter is it a problem to remove the protected sections of those records in the branch and maintenance version of Buildings that we are supporting? Otherwise, we can make it a warning, and then start the discussion with the MAP-Lang group, but we need a good motivation for that.

If the motivation was to reduce the amount of displayed stuff in the simulation results, you can use the HideResult annotation.

casella avatar May 16 '22 11:05 casella

If the motivation was to reduce the amount of displayed stuff in the simulation results, you can use the HideResult annotation.

I don't think that applies here, the protected element in this case is a record and not a component. I.e.:

record Generic
  ...
  parameter HeatingCoolingData hea;
  ...
protected
  record HeatingCoolingData
    ...
  end HeatingCoolingData;
end Generic;

perost avatar May 16 '22 11:05 perost

See Buildings #3008

casella avatar May 16 '22 13:05 casella

I will remove the protected keyword in this record definition through https://github.com/lbl-srg/modelica-buildings/issues/3009 To be fixed through https://github.com/lbl-srg/modelica-buildings/pull/3011 and https://github.com/lbl-srg/modelica-buildings/pull/3010 on master and maint_8.1.x branches.

mwetter avatar May 16 '22 13:05 mwetter