From 0b28b2ca7ada800333e66da119f6d42f0c2425ea Mon Sep 17 00:00:00 2001 From: Pedro Bressan Date: Thu, 23 Nov 2023 14:32:36 -0300 Subject: [PATCH 1/4] ENH: prevent out of bounds Tanks from construction. --- rocketpy/motors/tank.py | 106 +++++++++++++++++++++++++++++++--------- 1 file changed, 84 insertions(+), 22 deletions(-) diff --git a/rocketpy/motors/tank.py b/rocketpy/motors/tank.py index bba0ff7c9..4ff9dcfe7 100644 --- a/rocketpy/motors/tank.py +++ b/rocketpy/motors/tank.py @@ -110,7 +110,6 @@ def __init__(self, name, geometry, flux_time, liquid, gas, discretize=100): # Initialize plots and prints object self.prints = _TankPrints(self) self.plots = _TankPlots(self) - return None @property def flux_time(self): @@ -410,6 +409,73 @@ def inertia(self): """ return self.liquid_inertia + self.gas_inertia + def _check_volume_bounds(self): + """Checks if the tank is overfilled or underfilled. Raises a ValueError + if the tank is overfilled or underfilled. + """ + if (self.fluid_volume > self.geometry.total_volume + 1e-6).any(): + raise ValueError( + f"The tank '{self.name}' is overfilled. The fluid volume is " + + "greater than the total volume of the tank.\n\t\t" + + "Try increasing the tank height and check out the fluid density" + + " values.\n\t\t" + + f"The fluid volume is {np.max(self.fluid_volume.y_array):.3f} m³ at " + + f"{self.fluid_volume.x_array[np.argmax(self.fluid_volume.y_array)]:.3f} s." + ) + elif (self.fluid_volume < -1e-6).any(): + raise ValueError( + f"The tank '{self.name}' is underfilled. The fluid volume is " + + "negative.\n\t\t" + + "Try increasing input fluid quantities and check out the fluid density" + + " values.\n\t\t" + + f"The fluid volume is {np.min(self.fluid_volume.y_array):.3f} m³ at " + + f"{self.fluid_volume.x_array[np.argmin(self.fluid_volume.y_array)]:.3f} s." + ) + + def _check_height_bounds(self): + """Checks if the tank is overfilled or underfilled. Raises a ValueError + if the tank is overfilled or underfilled. + """ + top_tolerance = self.geometry.top + 1e-4 + bottom_tolerance = self.geometry.bottom - 1e-4 + + if (self.liquid_height > top_tolerance).any(): + raise ValueError( + f"The tank '{self.name}' is overfilled. " + + f"The liquid height is above the tank top.\n\t\t" + + "Try increasing the tank height and check out the fluid density" + + " values.\n\t\t" + + f"The liquid height is {np.max(self.liquid_height.y_array):.3f} m above " + + f"the tank top at {self.liquid_height.x_array[np.argmax(self.liquid_height.y_array)]:.3f} s." + ) + elif (self.liquid_height < bottom_tolerance).any(): + raise ValueError( + f"The tank '{self.name}' is underfilled. " + + "The liquid height is below the tank bottom.\n\t\t" + + "Try increasing input fluid quantities and check out the fluid density" + + " values.\n\t\t" + + f"The liquid height is {np.min(self.liquid_height.y_array):.3f} m below " + + f"the tank bottom at {self.liquid_height.x_array[np.argmin(self.liquid_height.y_array)]:.3f} s." + ) + elif (self.gas_height > top_tolerance).any(): + raise ValueError( + f"The tank '{self.name}' is overfilled. " + + "The gas height is above the tank top.\n\t\t" + + "Try increasing the tank height and check out the fluid density" + + " values.\n\t\t" + + f"The gas height is {np.max(self.gas_height.y_array):.3f} m above " + + f"the tank top at {self.gas_height.x_array[np.argmax(self.gas_height.y_array)]:.3f} s." + ) + elif (self.gas_height < bottom_tolerance).any(): + raise ValueError( + f"The tank '{self.name}' is underfilled. " + + "The gas height is below the tank bottom.\n\t\t" + + "Try increasing input fluid quantities and check out the fluid density" + + " values.\n\t\t" + + f"The gas height is {np.min(self.gas_height.y_array):.3f} m below " + + f"the tank bottom at {self.gas_height.x_array[np.argmin(self.gas_height.y_array)]:.3f} s." + ) + def draw(self): """Draws the tank geometry.""" self.plots.draw() @@ -535,7 +601,10 @@ def __init__( # Discretize input flow if needed self.discretize_flow() if discretize else None - return None + + # Check if the tank is overfilled or underfilled + self._check_volume_bounds() + self._check_height_bounds() @funcify_method("Time (s)", "Mass (kg)") def fluid_mass(self): @@ -814,15 +883,9 @@ def __init__( # Discretize input if needed self.discretize_ullage() if discretize else None - # Check if the ullage is within bounds - if (self.ullage > self.geometry.total_volume).any(): - raise ValueError( - "The ullage volume is out of bounds. It is greater than the " - + "total volume of the tank." - ) - if (self.ullage < 0).any(): - raise ValueError("The ullage volume is out of bounds. It is negative.") - return None + # Check if the tank is overfilled or underfilled + self._check_volume_bounds() + self._check_height_bounds() @funcify_method("Time (s)", "Mass (kg)") def fluid_mass(self): @@ -862,7 +925,7 @@ def fluid_volume(self): Function Volume of the fluid as a function of time. """ - return self.geometry.total_volume + return Function(self.geometry.total_volume).set_discrete_based_on_model(self.gas_volume) @funcify_method("Time (s)", "Volume (m³)") def liquid_volume(self): @@ -1012,13 +1075,9 @@ def __init__( # Discretize input if needed self.discretize_liquid_height() if discretize else None - # Check if the liquid level is within bounds - if (self.liquid_level > self.geometry.top).any(): - raise ValueError( - "The liquid level is out of bounds. It is greater than the tank top." - ) - if (self.liquid_level < self.geometry.bottom).any(): - raise ValueError("The liquid level is out of bounds. It is negative.") + # Check if the tank is overfilled or underfilled + self._check_volume_bounds() + self._check_height_bounds() @funcify_method("Time (s)", "Mass (kg)") def fluid_mass(self): @@ -1062,7 +1121,7 @@ def fluid_volume(self): Volume of the fluid as a function of time. """ volume = self.gas_volume + self.liquid_volume - diff = abs(volume - self.geometry.total_volume) + diff = volume - self.geometry.total_volume if (diff > 1e-6).any(): raise ValueError( "The `fluid_volume`, defined as the sum of `gas_volume` and " @@ -1096,9 +1155,8 @@ def gas_volume(self): Volume of the gas as a function of time. """ # TODO: there's a bug on the gas_center_of_mass is I don't discretize here - func = Function(self.geometry.total_volume) + func = Function(self.geometry.total_volume).set_discrete_based_on_model(self.liquid_volume) func -= self.liquid_volume - func.set_discrete_based_on_model(self.liquid_volume) return func @funcify_method("Time (s)", "Height (m)") @@ -1230,6 +1288,10 @@ def __init__( # Discretize input if needed self.discretize_masses() if discretize else None + # Check if the tank is overfilled or underfilled + self._check_volume_bounds() + self._check_height_bounds() + @funcify_method("Time (s)", "Mass (kg)") def fluid_mass(self): """ From 72d8c561bba7b1f6658fd6e9d19059113ea6b7fd Mon Sep 17 00:00:00 2001 From: Pedro Bressan Date: Thu, 23 Nov 2023 14:36:37 -0300 Subject: [PATCH 2/4] STY: apply black for linting. --- rocketpy/motors/tank.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/rocketpy/motors/tank.py b/rocketpy/motors/tank.py index 4ff9dcfe7..031fb4e45 100644 --- a/rocketpy/motors/tank.py +++ b/rocketpy/motors/tank.py @@ -925,7 +925,9 @@ def fluid_volume(self): Function Volume of the fluid as a function of time. """ - return Function(self.geometry.total_volume).set_discrete_based_on_model(self.gas_volume) + return Function(self.geometry.total_volume).set_discrete_based_on_model( + self.gas_volume + ) @funcify_method("Time (s)", "Volume (m³)") def liquid_volume(self): @@ -1155,7 +1157,9 @@ def gas_volume(self): Volume of the gas as a function of time. """ # TODO: there's a bug on the gas_center_of_mass is I don't discretize here - func = Function(self.geometry.total_volume).set_discrete_based_on_model(self.liquid_volume) + func = Function(self.geometry.total_volume).set_discrete_based_on_model( + self.liquid_volume + ) func -= self.liquid_volume return func From a17790a82ab1c7fd7e3e8b33d50e12339dec694a Mon Sep 17 00:00:00 2001 From: Pedro Bressan Date: Fri, 24 Nov 2023 11:22:11 -0300 Subject: [PATCH 3/4] MNT: improve error messages for Tank out of bounds filling. --- rocketpy/motors/tank.py | 99 ++++++++++++++++++++++------------------- 1 file changed, 54 insertions(+), 45 deletions(-) diff --git a/rocketpy/motors/tank.py b/rocketpy/motors/tank.py index 031fb4e45..d5df51b84 100644 --- a/rocketpy/motors/tank.py +++ b/rocketpy/motors/tank.py @@ -411,71 +411,80 @@ def inertia(self): def _check_volume_bounds(self): """Checks if the tank is overfilled or underfilled. Raises a ValueError - if the tank is overfilled or underfilled. + if either the `gas_volume` or `liquid_volume` are out of tank geometry + bounds. """ - if (self.fluid_volume > self.geometry.total_volume + 1e-6).any(): + + def overfill_volume_exception(param_name, param): raise ValueError( - f"The tank '{self.name}' is overfilled. The fluid volume is " + f"The tank '{self.name}' is overfilled. The {param_name} is " + "greater than the total volume of the tank.\n\t\t" - + "Try increasing the tank height and check out the fluid density" - + " values.\n\t\t" - + f"The fluid volume is {np.max(self.fluid_volume.y_array):.3f} m³ at " - + f"{self.fluid_volume.x_array[np.argmax(self.fluid_volume.y_array)]:.3f} s." + + "Try increasing the tank height and check out the fluid density " + + "values.\n\t\t" + + f"The {param_name} is {param.max:.3f} m³ at " + + f"{param.x_array[np.argmax(param.y_array)]:.3f} s.\n\t\t" + + f"The tank total volume is {self.geometry.total_volume:.3f} m³." ) - elif (self.fluid_volume < -1e-6).any(): + + def underfill_volume_exception(param_name, param): raise ValueError( - f"The tank '{self.name}' is underfilled. The fluid volume is " + f"The tank '{self.name}' is underfilled. The {param_name} is " + "negative.\n\t\t" - + "Try increasing input fluid quantities and check out the fluid density" - + " values.\n\t\t" - + f"The fluid volume is {np.min(self.fluid_volume.y_array):.3f} m³ at " - + f"{self.fluid_volume.x_array[np.argmin(self.fluid_volume.y_array)]:.3f} s." + + "Try increasing input fluid quantities and check out the fluid " + + "density values.\n\t\t" + + f"The {param_name} is {param.min:.3f} m³ at " + + f"{param.x_array[np.argmin(param.y_array)]:.3f} s.\n\t\t" + + f"The tank total volume is {self.geometry.total_volume:.3f} m³." ) + for name, volume in [ + ("gas volume", self.gas_volume), + ("liquid volume", self.liquid_volume), + ]: + if (volume > self.geometry.total_volume + 1e-6).any(): + overfill_volume_exception(name, volume) + elif (volume < -1e-6).any(): + underfill_volume_exception(name, volume) + def _check_height_bounds(self): """Checks if the tank is overfilled or underfilled. Raises a ValueError - if the tank is overfilled or underfilled. + if either the `gas_height` or `liquid_height` are out of tank geometry + bounds. """ top_tolerance = self.geometry.top + 1e-4 bottom_tolerance = self.geometry.bottom - 1e-4 - if (self.liquid_height > top_tolerance).any(): - raise ValueError( - f"The tank '{self.name}' is overfilled. " - + f"The liquid height is above the tank top.\n\t\t" - + "Try increasing the tank height and check out the fluid density" - + " values.\n\t\t" - + f"The liquid height is {np.max(self.liquid_height.y_array):.3f} m above " - + f"the tank top at {self.liquid_height.x_array[np.argmax(self.liquid_height.y_array)]:.3f} s." - ) - elif (self.liquid_height < bottom_tolerance).any(): - raise ValueError( - f"The tank '{self.name}' is underfilled. " - + "The liquid height is below the tank bottom.\n\t\t" - + "Try increasing input fluid quantities and check out the fluid density" - + " values.\n\t\t" - + f"The liquid height is {np.min(self.liquid_height.y_array):.3f} m below " - + f"the tank bottom at {self.liquid_height.x_array[np.argmin(self.liquid_height.y_array)]:.3f} s." - ) - elif (self.gas_height > top_tolerance).any(): + def overfill_height_exception(param_name, param): raise ValueError( f"The tank '{self.name}' is overfilled. " - + "The gas height is above the tank top.\n\t\t" - + "Try increasing the tank height and check out the fluid density" - + " values.\n\t\t" - + f"The gas height is {np.max(self.gas_height.y_array):.3f} m above " - + f"the tank top at {self.gas_height.x_array[np.argmax(self.gas_height.y_array)]:.3f} s." + + f"The {param_name} is above the tank top.\n\t\t" + + "Try increasing the tank height and check out the fluid density " + + "values.\n\t\t" + + f"The {param_name} is {param.max:.3f} m above the tank top " + + f"at {param.x_array[np.argmax(param.y_array)]:.3f} s.\n\t\t" + + f"The tank top is at {self.geometry.top:.3f} m." ) - elif (self.gas_height < bottom_tolerance).any(): + + def underfill_height_exception(param_name, param): raise ValueError( f"The tank '{self.name}' is underfilled. " - + "The gas height is below the tank bottom.\n\t\t" - + "Try increasing input fluid quantities and check out the fluid density" - + " values.\n\t\t" - + f"The gas height is {np.min(self.gas_height.y_array):.3f} m below " - + f"the tank bottom at {self.gas_height.x_array[np.argmin(self.gas_height.y_array)]:.3f} s." + + f"The {param_name} is below the tank bottom.\n\t\t" + + "Try increasing input fluid quantities and check out the fluid " + + "density values.\n\t\t" + + f"The {param_name} is {param.min:.3f} m below the tank bottom " + + f"at {param.x_array[np.argmin(param.y_array)]:.3f} s.\n\t\t" + + f"The tank bottom is at {self.geometry.bottom:.3f} m." ) + for name, height in [ + ("gas height", self.gas_height), + ("liquid height", self.liquid_height), + ]: + if (height > top_tolerance).any(): + overfill_height_exception(name, height) + elif (height < bottom_tolerance).any(): + underfill_height_exception(name, height) + def draw(self): """Draws the tank geometry.""" self.plots.draw() @@ -1078,8 +1087,8 @@ def __init__( self.discretize_liquid_height() if discretize else None # Check if the tank is overfilled or underfilled - self._check_volume_bounds() self._check_height_bounds() + self._check_volume_bounds() @funcify_method("Time (s)", "Mass (kg)") def fluid_mass(self): From 6d1d7bdb62a20a225ab2d8ffff10c15ff8510540 Mon Sep 17 00:00:00 2001 From: MateusStano Date: Fri, 24 Nov 2023 17:39:47 +0100 Subject: [PATCH 4/4] DOC: add to changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f8a6cd6aa..ad1a6324b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,7 +33,7 @@ straightforward as possible. ### Added - DOC: Added this changelog file [#472](https://github.com/RocketPy-Team/RocketPy/pull/472) -- +- ENH: Prevent out of bounds Tanks from Instantiation #484 [#484](https://github.com/RocketPy-Team/RocketPy/pull/484) ### Changed