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

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

Open
casella opened this issue Jun 19, 2024 · 2 comments
Open

Comments

@casella
Copy link

casella commented Jun 19, 2024

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

replaceable package Gas =
ThermofluidStream.Media.myMedia.Air.DryAirNasa;
replaceable package Liquid =
ThermofluidStream.Media.myMedia.Incompressible.Examples.JP8;

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

redeclare package Gas =
ThermofluidStream.Media.myMedia.Air.DryAirNasa;
redeclare package Liquid =
ThermofluidStream.Media.myMedia.Incompressible.Examples.JP8;

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

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 casella changed the title Liquid and Gas sub-packages should probably be replaceable Liquid and Gas sub-packages should probably have more general base classes Jun 24, 2024
@casella
Copy link
Author

casella commented Jun 24, 2024

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 😃

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

Successfully merging a pull request may close this issue.

2 participants