search

ibpsa / modelica-ibpsa

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

medium package for propylene-glycol / water mixtures #913

Closed MassimoCimmino closed 6 years ago

MassimoCimmino commented 6 years ago

This follows the discussion from the 2018-04-26 library development meeting.

This issue is to implement a medium package for propylene-glycol antifreeze mixtures. It should behave the same as IBPSA.Media.Water, meaning the properties (thermal capacity, thermal conductivity, density and viscosity) are constant and evaluated at a reference temperature and given mass fraction of antifreeze.

The model is already implemented in branch 422b (#422) under IBPSA.Media.SecondaryFluid.PropyleneGlycolWater. It was suggested to rename to IBPSA.Media.Antifreeze.PropyleneGlycolWater.

If there is interest, other fluids may follow: ethylene-glycol, ethyl-alcohol and methyl-alcohol.

mwetter commented 6 years ago

There certainly is interest in the other media.

mwetter commented 6 years ago

@MassimoCimmino : The implementation looks pretty clean. I will need some more time to review, but can you please address these:

Mathadon commented 6 years ago

@MassimoCimmino sounds great :)

MassimoCimmino commented 6 years ago

Thank you @mwetter. I will look into this very soon.

For property_T and reference_T, I decided to have 2 variables as reference_T is used in the calculation of enthalpy. It is the temperature of zero enthalpy.

mwetter commented 6 years ago

@MassimoCimmino : I agree, in this case it makes sense to introduce property_T.

mwetter commented 6 years ago

@MassimoCimmino : It looks like the JModelica tests are failing because the file PropyleneGlycolWater.mo contains not only the package PropyleneGlycolWater, but also the package BaseClasses. The Modelica Language definition states:

When mapping a package or class-hierarchy to a file [e.g. the file A.mo], that file shall only define a single class [A] with a name matching the name of the nonstructured entity.

Let me know if I should look into refactoring it. Best would probably be to make the functions that are now in BaseClasses protected. Then the media can still be replaced with another one.

MassimoCimmino commented 6 years ago

@mwetter : I am working on a fix.

I removed the functions pertaining to individual properties (density, conductivity, ...) and placed the coefficients for the correlations in a record, as it is done for the fluid constants.

I am working on the branch issue913_antifreezePropyleneGlycolWater_refactor. I will see if Travis is able to run the JModelica tests and create a pull request afterwards.

MassimoCimmino commented 6 years ago

issue913_antifreezePropyleneGlycolWater_refactor now passes the tests. The issue was incorrect declaration and use of the constant records that contain the fluid properties. I will go through the models one last time before merging this into the main issue913 branch.

mwetter commented 6 years ago

@MassimoCimmino Thanks for the update. Glad to read that you resolved the problem.

MassimoCimmino commented 6 years ago

@mwetter I have updated #914 and it now passes all of the tests.

mwetter commented 6 years ago

@MassimoCimmino : I reviewed your implementation and revised it for consistency with the other media and the naming convention. I also deleted an old example Validation.PolynomialProperty as this function is now in the medium. This example was not part of the unit tests (as it lacked an .mos script).

Please let me know if you are fine with the changes. From my point of view, this would be ready to be merged.

MassimoCimmino commented 6 years ago

@mwetter : I have made a small change to the documentation of Antifreeze.BaseClasses.PropertyCoefficients as it was missing words.

The changes look very good. I see you decided not to have the fluid property functions become protected. Is this fine as it is now?

mwetter commented 6 years ago

@MassimoCimmino : I just change the implementation to make them protected. I prefer having them protected as they are not needed by the user, and other media do not expose such functions either. This however required adding a new media IBPSA.Media.Antifreeze.Validation.BaseClasses.PropyleneGlycolWater that extends IBPSA.Media.Antifreeze.PropyleneGlycolWater in order to make these functions public so that they can be validated in IBPSA.Media.Antifreeze.Validation.BaseClasses.FluidProperties. This was needed because protected functions can be accessed by a class that extends them, but not by a class that instantiates them.

From my point of view, this is ready to be merged, but as I made a lot of changes, it is best if someone independent such as @Mathadon can have a last review before we merge it to the master. Please let me know if you have other changes or disagree with my latest revision.

MassimoCimmino commented 6 years ago

I do not mind the changes. I hope I didn't make you work too hard.

nytschgeusen commented 6 years ago

We are using IBPSA.Media.Antifreeze.PropyleneGlycolWater in our BuildingSystems library as the medium in the collector loop of our solar thermal systems, e.g. in the example https://github.com/UdK-VPT/BuildingSystems/blob/master/BuildingSystems/Applications/SolarThermalSystems/SolarThermalSystem1.mo .

Thank you, for the implementation of this medium.

The problem with the current implementation is that the temperature in the primary loops of solar thermal systems can reach definitely more than 100 °C (e.g outlet temperature of the collector up to 180 °C can occur during stagnation phases).

At the moment the maximum possible temperature of IBPSA.Media.Antifreeze.PropyleneGlycolWater is restricted to 100 °C (see https://github.com/ibpsa/modelica-ibpsa/blob/master/IBPSA/Media/Antifreeze/PropyleneGlycolWater.mo):

in line 15:

final T_max=Modelica.SIunits.Conversions.from_degC(100),

It would be great if the final could be removed for future versions 😀. We did it at the moment for the integrated IBPSA library in our BuildingSystems library manually, but perhaps other people will have similar problems if they use the PropyleneGlycolWater medium in a higher temperature context.

Mathadon commented 6 years ago

+1

mwetter commented 6 years ago

It's OK to remove the final. Can you please make a pull request. Thanks

MassimoCimmino commented 6 years ago

I agree. Note that the implemented correlations for the physical properties are produced from experimental measurements up to the temperature T_max.