ENH: Allow for time variable density to Tank Fluids. #593
Conversation
phmbressan
added
the
Enhancement
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #593 +/- ##
========================================
Coverage 73.03% 73.03%
========================================
Files 57 57
Lines 9596 9609 +13
========================================
+ Hits 7008 7018 +10
- Misses 2588 2591 +3 ☔ View full report in Codecov by Sentry. |
| density : int, float, callable, string, array, Function | ||
| Density of the fluid in kg/m³. If a int or float is given, | ||
| it is considered a constant in time. A callable, csv file | ||
| or an list of points can be given to express the density | ||
| as a function of time. The parameter is used as a ``Function`` | ||
| source. |
There was a problem hiding this comment.
I believe it makes no sense to have a fluid density depending on the time. Please correct me if I'm wrong, but how can someone predict this function in the first place?
Wouldn't be more appropiate to have this density parameter depending on the temperature and pressure? To create a function like this:
def density(T, P):
# so some interpolation
return valueThis could be created with the assistance of 2D Function objects and .csv files.
We could even include some "common" fluids csv files in the package, so the user could just call the function with the fluid name and the function would load the csv file and return the 2D Function object.
In case the user still wants to use a constant value, it would still be possible.
There was a problem hiding this comment.
Thanks for your suggestion, I considered it at first, but I was hesitant about it for the following reasons. The Tanks module currently does not calculate any thermodynamical quantities.
Correct me if I am mistaken, but unless the Temperature and Pressure in your comment depend on time, we cannot compute anything with it.
At this point, if the Temperature and Pressure are know time variable quantities, the density is also known as a time variable quantity.
Furthermore, the Pressure and Temperature are not used anywhere is Tanks simulation, we would be introducing two new quantities that are not necessary.
I can see two ways of getting the Fluid density in a static fire:
- Having sensors that compute two thermodynamical properties (such as temperature and pressure) and inferring the others from thermodynamical models;
- Having sensors for mass flow and fluid level inside the tank, then the density can be computed.
In my view, in both ways it rather easy to get the density as a function of time. If we implement it from pressure and temperature, we limit ourserves to a particular case (but a really common one) of the first option.
I understand what you mean by having the common Fluids in a file and we making the conversion, but I believe this step into thermodynamical territory and this is a bit against our initial intention for the Tank class. Of course, there is always a place to start improving and adding new features/ideas.
Let me know your opinion on the matter.
There was a problem hiding this comment.
Can we make Fluid objects motor-agnostic? If fluid's properties depend on time, this time might be associated with a particular motor, so the same fluid couldn't be used in other tanks.
I was thinking about this:
- You have the volume of the tank, this is always a fixed value.
- By each time, you calculate the mass of fluid multiplying the volume by the density,
- This density would actually depend on pressure and temperature.
I know that different types of liquid motors will be defined based on different types of input (liquid level, pressure, mass, etc.), but that would be something to be carefully worked on.
What really complicates the whole thing is that we have both liquids and gases defined as a Fluid, but those may present quite different behaviors.
| density: Union[int, float, str, list, Function] | ||
|
|
There was a problem hiding this comment.
Python functions could also be accepted, right? (for example the lambda you applied in the example)
I would let callable be a valid type for the density parameter instead of just rocketpy.Function
| density: Union[int, float, str, list, Function] | |
| density: Union[int, float, str, list, callable] | |
| def _discretize_fluids(self): | ||
| """Discretizes the fluid densities according to the flux time and the |
There was a problem hiding this comment.
| def _discretize_fluids(self): | |
| """Discretizes the fluid densities according to the flux time and the | |
| def _discretize_fluidd_density(self): | |
| """Discretizes the fluid densities according to the flux time and the |
There was a problem hiding this comment.
Much better, thanks for the suggestion, I will implement it.
Pull request type
Checklist
black rocketpy/ tests/) has passed locallypytest tests -m slow --runslow) have passed locallyCHANGELOG.mdhas been updated (if relevant)Current behavior
Currently, the
Tankclass assumes in its computations that its fluid densities are a constant in time. This approximation can be rather inaccurate, specially for tanks that experiment large variations in fluid density (e.g. due to changes in temperature and pressure) during the flow.New behavior
This PR essentially allows for time variable densities when defining the
Fluidand theTanksubclasses discretize these values in time for code speed.Breaking change
Additional Information
The tests for the
Tankclass would benefit a lot from a refactor. However, I preferred to keep this PR focused on the new functionalities.