ibpsa / modelica-ibpsa

Modelica library for building and district energy systems developed within IBPSA Project 1
https://ibpsa.github.io/project1
140 stars 83 forks source link

Pedantic Modelica #116

Closed thorade closed 9 years ago

thorade commented 9 years ago

When checking the code with Dymola 2015 in pedantic mode (i.e. with flag Advanced.PedanticModelica=true) there are some errors. This is issue can be used to collect fixes for these errors.

thorade commented 9 years ago

What does annotation(Window()) do? Should we keep it? Also asked the same question on SO: http://stackoverflow.com/q/26671901/874701

thorade commented 9 years ago

Annex60.Fluid.BaseClasses.FlowModels.basicFlowFunction_dp and Annex60.Fluid.BaseClasses.PartialResistance each have multiple annotation(revisions, all of them by @mwetter. Could you combine them into one single revision list per file and make a check of Annex60.Fluid.BaseClasses afterwards with Advanced.PedanticModelica=true?

thorade commented 9 years ago

There is an error message about redeclaring ThermodynamicState. I am unsure how this should be fixed, first I thought this has to be fixed in Modelica.Media and filed an issue here: https://trac.modelica.org/Modelica/ticket/1599 but that breaks some other things. Maybe there is a different way to set good start values?

thorade commented 9 years ago

There is an error message

Use of undeclared variable med.phi In class Annex60.Fluid.Sensors.RelativeHumidity.

Who knows how to fix that?

Mathadon commented 9 years ago

You can fix it by redeclaring the medium into a medium that contains moisture. However I think this removes the parameter box and this is probably the reason why it is not done right now.

Mathadon commented 9 years ago

With 'parameter box' I mean the option to set the medium using the drop down menu in the parameter box of the model.

thorade commented 9 years ago

OK. Are there any media apart from Annex60.Media.Air that have phi in their BaseProperties? Would it be cleaner to define a partial medium that adds phi to the BaseProperties and let Air extend from that partial medium?

mwetter commented 9 years ago

@thorade The cleanest would probably be to implement a model like Annex60.Utilities.Psychrometrics.X_pTphi but that has phi as an output, and then use this model in the sensor.

Adding a partial medium won't help much as none of the Modelica.Media models would work (should users desire to use them) because they don't extend from this partial medium.

mwetter commented 9 years ago

@thorade what file is annotation(Window()) in?

thorade commented 9 years ago

Sorry, my fault. The annotation(Window()) is not present in Annex60, only in our BuildingSystems library, in the following files: BuildingSystems.Utilities.Math.BooleanReplicator BuildingSystems.Utilities.Math.IntegerReplicator BuildingSystems.Fluid.Sources.MassFlowSource_T BuildingSystems.Fluid.Sources.MassFlowSource_h. These files were integrated into our library from Annex60, maybe that is the reason why I thought the annotation came from Annex60. I think we should run the integration scripts again and see where the annotation was introduced.

Mathadon commented 9 years ago

Actually redeclaring the Media should not pose a problem as longs as you use redeclare replaceable packageinstead of redeclare package. So my previous comment was wrong.

thorade commented 9 years ago

Closing for now, parts have been taken care of with #118, other parts are now discussed in #123 and #124 and #128 and #148.