search

DLR-SR / ThermofluidStream

The DLR Thermofluid Stream Library
BSD 3-Clause "New" or "Revised" License
60 stars 27 forks source link

Liquid and Gas sub-packages should probably have more general base classes #193

Open casella opened 3 months ago

casella commented 3 months ago

The ThermofluidStream.Media.myMedia.GasAndIncompressible.PartialGasAndIncompressible class contains the following code:

https://github.com/DLR-SR/ThermofluidStream/blob/38553ffdb79f8d65566b06c46249a14605eca1e9/ThermofluidStream/Media/myMedia/GasAndIncompressible/PartialGasAndIncompressible.mo#L17-L20

This works fine as long as there is only one medium JP8DryAir that extends this base class and has the same redeclares

https://github.com/DLR-SR/ThermofluidStream/blob/38553ffdb79f8d65566b06c46249a14605eca1e9/ThermofluidStream/Media/myMedia/GasAndIncompressible/JP8DryAir/package.mo#L16-L19

but in general, the PartialGasAndIncompressible class definition should contain abstract base classes for Liquid and Gas; since the documentation of the code says that currently only pure substances are allowed for both, I guess the code in PartialGasAndIncompressible should be like

 replaceable package Gas = 
     ThermofluidStream.Media.myMedia.Interfaces.PartialPureSubstance;
 replaceable package Liquid = 
     ThermofluidStream.Media.myMedia.Interfaces.PartialPureSubstance;
IngelaLind commented 3 months ago

The above suggestion does not work. The least restrictive base classes that still work without giving other problems are replaceable package Gas = ThermofluidStream.Media.myMedia.IdealGases.Common.SingleGasNasa; replaceable package Liquid = ThermofluidStream.Media.myMedia.Incompressible.TableBased;

The main offender if a more general class selection is wanted is the computation of R_s in the BaseProperties.

I have problems figuring out where I should suggest the changes in GitHub. Totally confused by all the forks and branches actually, so I have just focused on understanding the problem at the moment.

casella commented 3 months ago

OK. Ultimately which base class you pick is up to you. This ticket was just to point out that the currently declared classes are replaceable in name only, but you can only redeclare them to sub-classes of JP8 and DryAirNasa, which is a very small (basically empty) sub-set of medium models. SingleGasNasa and TableBased makes a lot more sense.

That said, this issue is only potential at the moment; as long as you only have one concrete medium implementation, it doesn't really matter. I suggest to only change it in the main branch for future versions and forget about forks and branches 😃