open-ideas / IDEAS

Modelica library allowing simultaneous transient simulation of thermal and electrical systems at both building and feeder level.
131 stars 56 forks source link

Violations of protected #1320

Open maltelenz opened 1 year ago

maltelenz commented 1 year ago

Validating models in Wolfram System Modeler, I get errors about violations of protected in multiple classes, as reported below:

IDEAS.Buildings.Components.BaseClasses.ConvectiveHeatTransfer.Examples.CavityAirflow:

Error: IDEAS.Buildings.Components.BaseClasses.ConvectiveHeatTransfer.Examples.CavityAirflow [4:3-4:3] Invalid lookup of protected element. When looking up cavityAirFlow.G in scope IDEAS.Buildings.Components.BaseClasses.ConvectiveHeatTransfer.Examples.CavityAirflow, G is protected. Protected elements can only appear as the first part of a component reference or path, and cannot be imported.

IDEAS.Experimental.Electric.Examples.TestGridAndPVFromFile:

Error: IDEAS.Experimental.Electric.Distribution.AC.Grid_3P [19:3-19:3] Invalid lookup of protected element. When looking up gridOnly3P.Vabs in scope IDEAS.Experimental.Electric.Distribution.AC.Grid_3P, Vabs is protected. Protected elements can only appear as the first part of a component reference or path, and cannot be imported.

IDEAS.Experimental.Electric.Examples.TestGridAndPVSystemGeneral:

Error: IDEAS.Experimental.Electric.Photovoltaics.Components.PvArray [34:3-34:3] Invalid lookup of protected element. When looking up radSol.weaBus in scope IDEAS.Experimental.Electric.Photovoltaics.Components.PvArray, weaBus is protected. Protected elements can only appear as the first part of a component reference or path, and cannot be imported. Error: IDEAS.Experimental.Electric.Distribution.AC.Grid_3P [19:3-19:3] Invalid lookup of protected element. When looking up gridOnly3P.Vabs in scope IDEAS.Experimental.Electric.Distribution.AC.Grid_3P, Vabs is protected. Protected elements can only appear as the first part of a component reference or path, and cannot be imported.

IDEAS.LIDEAS.Validation.Case900ValidationLinearInputs:

Error: IDEAS.LIDEAS.Validation.Case900ValidationNonLinear [22:3-22:3] Invalid lookup of protected element. When looking up linRecZon.airModel.vol.T in scope IDEAS.LIDEAS.Validation.Case900ValidationNonLinear, vol is protected. Protected elements can only appear as the first part of a component reference or path, and cannot be imported.

jelgerjansen commented 1 year ago

Although I don't see an actual problem in using the protected variable in these example models, it's true that Modelica does not allow the use of protected variables outside the model where it is defined and therefore we could try to circumvent this somehow. I see two (straightforward) solutions for this:

  1. Create a public variable that takes the value for the existing protected variable in the component models.
  2. Make the protected variable a final parameter, then the variable cannot be modified or overridden, but it remains accessible from the outside.

However, I already noticed this happens in quite some other components, so it will probably require a thorough review to implement this everywhere. @Mathadon, what's your opinion on this?

Mathadon commented 1 year ago

This is a recurring problem. We're using variables that are protected in specific situations where it makes sense to use them for development purposes but where we do not want these variables to show up in every user's result file. In the CavityAirFlow it would make sense to unprotect the variable. In IDEAS.Experimental I'd ignore it since these models are not actively maintained. In IDEAS.LIDEAS it might make sense to unprotect vol or make vol.T accessible through a new output.

I think it's a developer choice at the moment whether or not Examples should strictly follow the Modelica spec. However, for non-example (components) I'd recommend to follow the spec as much as possible.

maltelenz commented 1 year ago

I think it's a developer choice at the moment whether or not Examples should strictly follow the Modelica spec.

Of course it is always up to the library developer to choose how to develop their libraries.

As a Modelica tool developer that wants to figure out if we support a certain library, the examples in the library are often the main way to test this, especially for the vast majority of libraries where there is no test library available.

If the examples do not work because of illegal Modelica, our message to our customers then has to be "we can't say we support this library because it uses illegal Modelica". And of course people who try the library by trying to run the examples will get the (correct) impression that the library is broken, since the examples won't build.

Mathadon commented 1 year ago

Indeed, the examples are quite important in that respect.. Unfortunately this library is developed by volunteers so bandwidth is a bit limited to polish things :) But I'll leave it up to the active developers to decide how to proceed!

jelgerjansen commented 1 year ago

I will discuss with the other active developers whether or not we can find the time to fix the 'incorrect' Modelica spec. (@JavierArroyoBastida, @lucasverleyen, FYI)

@maltelenz the examples in the IDEAS.Experimental package can be neglected, since we do not actively use and/or support these models. In the future we might change that name to IDEAS.Obsolete. Feel free to address any other problems in the meantime (like #1321, I'll come back to that later).

maltelenz commented 1 year ago

@maltelenz the examples in the IDEAS.Experimental package can be neglected

Thank you, this is helpful information in our testing.