Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Violations of protected #1320

Open
maltelenz opened this issue May 9, 2023 · 6 comments
Open

Violations of protected #1320

maltelenz opened this issue May 9, 2023 · 6 comments

Comments

@maltelenz
Copy link

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
Copy link
Contributor

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
Copy link
Member

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
Copy link
Author

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
Copy link
Member

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
Copy link
Contributor

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
Copy link
Author

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

Thank you, this is helpful information in our testing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants