-
-
Notifications
You must be signed in to change notification settings - Fork 4
MNT: adapt discretization schema to RocketPy encoder. #52
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
Changes from 13 commits
9339a43
fb23890
ba7c28b
7c7f805
6ecc358
5eab472
8450082
c295651
42afc99
4368487
26f510d
52e09ca
a15232d
a8862c4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,9 +3,9 @@ | |
| import dill | ||
|
|
||
| from rocketpy.environment.environment import Environment as RocketPyEnvironment | ||
| from rocketpy.utilities import get_instance_attributes | ||
| from src.models.environment import EnvironmentModel | ||
| from src.views.environment import EnvironmentSimulation | ||
| from src.utils import collect_simulation_attributes | ||
|
|
||
|
|
||
| class EnvironmentService: | ||
|
|
@@ -54,8 +54,11 @@ def get_environment_simulation(self) -> EnvironmentSimulation: | |
| EnvironmentSimulation | ||
| """ | ||
|
|
||
| attributes = get_instance_attributes(self.environment) | ||
| env_simulation = EnvironmentSimulation(**attributes) | ||
| encoded_attributes = collect_simulation_attributes( | ||
| self.environment, | ||
| EnvironmentSimulation, | ||
| ) | ||
| env_simulation = EnvironmentSimulation(**encoded_attributes) | ||
| return env_simulation | ||
|
Comment on lines
+57
to
62
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Passing EnvironmentSimulation to collect_attributes is a no-op for a bare Environment object. collect_attributes tries obj.env for EnvironmentSimulation; with an Environment instance, that path is skipped. Rely on rocketpy_encoder directly or update the utility to handle both Flight and Environment inputs. Apply this diff: - encoded_attributes = collect_attributes(
- self.environment,
- [EnvironmentSimulation],
- )
+ encoded_attributes = collect_attributes(self.environment)Optionally, harden collect_attributes to support both patterns: # utils.py (illustrative)
elif issubclass(attribute_class, EnvironmentSimulation):
environment_attributes_list = [...]
try:
src = getattr(obj, "env", obj) # works for Flight or Environment
for key in environment_attributes_list:
if key not in attributes.get("env", {}):
try:
value = getattr(src, key)
if "env" not in attributes and src is not obj:
attributes["env"] = {}
(attributes["env"] if src is not obj else attributes)[key] = value
except AttributeError:
pass
except Exception:
pass🤖 Prompt for AI Agents |
||
|
|
||
| def get_environment_binary(self) -> bytes: | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -6,7 +6,6 @@ | |||||||||||||||
| from rocketpy.motors.solid_motor import SolidMotor | ||||||||||||||||
| from rocketpy.motors.liquid_motor import LiquidMotor | ||||||||||||||||
| from rocketpy.motors.hybrid_motor import HybridMotor | ||||||||||||||||
| from rocketpy.utilities import get_instance_attributes | ||||||||||||||||
| from rocketpy import ( | ||||||||||||||||
| LevelBasedTank, | ||||||||||||||||
| MassBasedTank, | ||||||||||||||||
|
|
@@ -18,6 +17,7 @@ | |||||||||||||||
| from src.models.sub.tanks import TankKinds | ||||||||||||||||
| from src.models.motor import MotorKinds, MotorModel | ||||||||||||||||
| from src.views.motor import MotorSimulation | ||||||||||||||||
| from src.utils import collect_simulation_attributes | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| class MotorService: | ||||||||||||||||
|
|
@@ -140,8 +140,11 @@ def get_motor_simulation(self) -> MotorSimulation: | |||||||||||||||
| Returns: | ||||||||||||||||
| MotorSimulation | ||||||||||||||||
| """ | ||||||||||||||||
| attributes = get_instance_attributes(self.motor) | ||||||||||||||||
| motor_simulation = MotorSimulation(**attributes) | ||||||||||||||||
| encoded_attributes = collect_simulation_attributes( | ||||||||||||||||
| self.motor, | ||||||||||||||||
| MotorSimulation, | ||||||||||||||||
|
||||||||||||||||
| self.motor, | |
| MotorSimulation, | |
| self.motor, | |
| None, # flight_simulation_class | |
| None, # rocket_simulation_class | |
| MotorSimulation, | |
| None, # environment_simulation_class |
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -11,14 +11,14 @@ | |||||||||||||||
| Fins as RocketPyFins, | ||||||||||||||||
| Tail as RocketPyTail, | ||||||||||||||||
| ) | ||||||||||||||||
| from rocketpy.utilities import get_instance_attributes | ||||||||||||||||
|
|
||||||||||||||||
| from src import logger | ||||||||||||||||
| from src.models.rocket import RocketModel, Parachute | ||||||||||||||||
| from src.models.sub.aerosurfaces import NoseCone, Tail, Fins | ||||||||||||||||
| from src.services.motor import MotorService | ||||||||||||||||
| from src.views.rocket import RocketSimulation | ||||||||||||||||
|
|
||||||||||||||||
| from src.views.motor import MotorSimulation | ||||||||||||||||
| from src.utils import collect_simulation_attributes | ||||||||||||||||
|
|
||||||||||||||||
| class RocketService: | ||||||||||||||||
| _rocket: RocketPyRocket | ||||||||||||||||
|
|
@@ -107,8 +107,12 @@ def get_rocket_simulation(self) -> RocketSimulation: | |||||||||||||||
| Returns: | ||||||||||||||||
| RocketSimulation | ||||||||||||||||
| """ | ||||||||||||||||
| attributes = get_instance_attributes(self.rocket) | ||||||||||||||||
| rocket_simulation = RocketSimulation(**attributes) | ||||||||||||||||
| encoded_attributes = collect_simulation_attributes( | ||||||||||||||||
| self.rocket, | ||||||||||||||||
| RocketSimulation, | ||||||||||||||||
| MotorSimulation, | ||||||||||||||||
|
||||||||||||||||
| RocketSimulation, | |
| MotorSimulation, | |
| RocketSimulation, | |
| MotorSimulation, | |
| EnvironmentSimulation, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Attribute backfill via collect_attributes is ineffective for a Rocket instance.
The helper looks for obj.rocket / obj.rocket.motor; with a Rocket input this branch is skipped. Let rocketpy_encoder drive encoding directly, or adjust the utility to detect object shape.
Apply this diff:
- encoded_attributes = collect_attributes(
- self.rocket,
- [RocketSimulation, MotorSimulation]
- )
+ encoded_attributes = collect_attributes(self.rocket)If you prefer keeping class hints, update utils similarly to handle src = getattr(obj, "rocket", obj) and src = getattr(getattr(obj, "rocket", obj), "motor", getattr(obj, "motor", None)).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| encoded_attributes = collect_attributes( | |
| self.rocket, | |
| [RocketSimulation, MotorSimulation] | |
| ) | |
| rocket_simulation = RocketSimulation(**encoded_attributes) | |
| encoded_attributes = collect_attributes(self.rocket) | |
| rocket_simulation = RocketSimulation(**encoded_attributes) |
🤖 Prompt for AI Agents
In src/services/rocket.py around lines 110 to 114, collect_attributes is
currently ineffective for a Rocket instance because it expects obj.rocket /
obj.rocket.motor and skips when passed a Rocket; change the call to let
rocketpy_encoder handle encoding directly or update the utility to detect shape:
when implementing the fix, either (A) pass the original Rocket to
rocketpy_encoder instead of using collect_attributes so encoding is driven by
the encoder, or (B) modify collect_attributes to use src = getattr(obj,
"rocket", obj) and motor_src = getattr(src, "motor", getattr(obj, "motor",
None)) so it correctly backfills attributes for both wrapper and direct Rocket
instances, preserving existing class hints.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,48 +1,127 @@ | ||
| # fork of https://github.com/encode/starlette/blob/master/starlette/middleware/gzip.py | ||
| import gzip | ||
| import io | ||
| import logging | ||
| import json | ||
|
|
||
| from typing import Annotated, NoReturn, Any | ||
| import numpy as np | ||
| from typing import NoReturn | ||
|
|
||
| from pydantic import PlainSerializer | ||
| from rocketpy._encoders import RocketPyEncoder | ||
| from starlette.datastructures import Headers, MutableHeaders | ||
| from starlette.types import ASGIApp, Message, Receive, Scope, Send | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
| def to_python_primitive(v: Any) -> Any: | ||
|
|
||
| def rocketpy_encoder(obj): | ||
| """ | ||
| Convert complex types to Python primitives. | ||
| Encode a RocketPy object using official RocketPy encoders. | ||
|
|
||
| This function uses RocketPy's official RocketPyEncoder for complete | ||
| object serialization. | ||
|
|
||
| Args: | ||
| v: Any value, particularly those with a 'source' attribute | ||
| containing numpy arrays or generic types. | ||
| obj: RocketPy object (Environment, Motor, Rocket, Flight) | ||
|
|
||
| Returns: | ||
| The primitive representation of the input value. | ||
| Dictionary of encoded attributes | ||
| """ | ||
| if hasattr(v, "source"): | ||
| if isinstance(v.source, np.ndarray): | ||
| return v.source.tolist() | ||
|
|
||
| if isinstance(v.source, (np.generic,)): | ||
| return v.source.item() | ||
|
|
||
| return str(v.source) | ||
|
|
||
| if isinstance(v, (np.generic,)): | ||
| return v.item() | ||
|
|
||
| if isinstance(v, (np.ndarray,)): | ||
| return v.tolist() | ||
| json_str = json.dumps( | ||
| obj, | ||
| cls=RocketPyEncoder, | ||
| include_outputs=True, | ||
| include_function_data=True, | ||
| discretize=True, | ||
| allow_pickle=False, | ||
| ) | ||
| return json.loads(json_str) | ||
|
|
||
| return str(v) | ||
|
|
||
|
|
||
| AnyToPrimitive = Annotated[ | ||
| Any, | ||
| PlainSerializer(to_python_primitive), | ||
| ] | ||
| def collect_simulation_attributes(flight_obj, flight_simulation_class, rocket_simulation_class, motor_simulation_class, environment_simulation_class): | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would rename Besides that, it seems that at points this function's parameters are used in the wrong order: e.g. a
def collect_attributes(obj, attribute_classes=None):
if attribute_classes is None:
attribute_classes = [] # Default empty list
for attribute_class in attribute_classes:
if issubclass(attribute_class, MotorSimulation):
# for loop for the motor attr
elif issubclass(attribute_class, RocketSimulation):
# ...When using it, things would work like: collect_attributes(self.rocket, [RocketSimulation, MotorSimulation])
def collect_attributes(obj, **kwargs):
motor_simulation_class = kwargs.get("motor_simulation_class", None)
... # All other parameters of other simulation classes
if motor_simulation_class is not None:
# for loop for getting the motor attr
... # Other for loopsWhen calling it, things would be similar: collect_attributes(self.motor, motor_simulation_class=MotorSimulation)I have a slight preference for the first way, but whatever fits and works better should do it. |
||
| """ | ||
| Collect attributes from various simulation classes and populate them from the flight object. | ||
|
|
||
| Args: | ||
| flight_obj: RocketPy Flight object | ||
| flight_simulation_class: FlightSimulation class | ||
| rocket_simulation_class: RocketSimulation class | ||
| motor_simulation_class: MotorSimulation class | ||
| environment_simulation_class: EnvironmentSimulation class | ||
|
|
||
| Returns: | ||
| Dictionary with all collected attributes | ||
| """ | ||
| attributes = rocketpy_encoder(flight_obj) | ||
|
|
||
| flight_attributes_list = [ | ||
| attr for attr in flight_simulation_class.__annotations__.keys() | ||
| if attr not in ['message', 'rocket', 'env'] | ||
| ] | ||
|
|
||
| rocket_attributes_list = [ | ||
| attr for attr in rocket_simulation_class.__annotations__.keys() | ||
| if attr not in ['message', 'motor'] | ||
| ] | ||
|
|
||
| motor_attributes_list = [ | ||
| attr for attr in motor_simulation_class.__annotations__.keys() | ||
| if attr not in ['message'] | ||
| ] | ||
|
|
||
| environment_attributes_list = [ | ||
| attr for attr in environment_simulation_class.__annotations__.keys() | ||
| if attr not in ['message'] | ||
| ] | ||
|
|
||
| try: | ||
| for key in flight_attributes_list: | ||
| try: | ||
| value = getattr(flight_obj, key) | ||
| attributes[key] = value | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe that if the attribute, i.e., the if not key in attributes:
attributes[key] = getattr(flight_obj, key) |
||
| except AttributeError as e: | ||
| logger.warning(f"Flight attribute '{key}' not found: {e}") | ||
| except Exception as e: | ||
| logger.error(f"Error getting flight attribute '{key}': {type(e).__name__}: {e}") | ||
| except Exception as e: | ||
| logger.error(f"Error processing flight attributes: {type(e).__name__}: {e}") | ||
|
|
||
| try: | ||
| for key in rocket_attributes_list: | ||
| try: | ||
| value = getattr(flight_obj.rocket, key) | ||
| attributes["rocket"][key] = value | ||
| except AttributeError as e: | ||
| logger.warning(f"Rocket attribute '{key}' not found: {e}") | ||
| except Exception as e: | ||
| logger.error(f"Error getting rocket attribute '{key}': {type(e).__name__}: {e}") | ||
| except Exception as e: | ||
| logger.error(f"Error processing rocket attributes: {type(e).__name__}: {e}") | ||
|
|
||
| try: | ||
| for key in motor_attributes_list: | ||
| try: | ||
| value = getattr(flight_obj.rocket.motor, key) | ||
| attributes["rocket"]["motor"][key] = value | ||
| except AttributeError as e: | ||
| logger.warning(f"Motor attribute '{key}' not found: {e}") | ||
| except Exception as e: | ||
| logger.error(f"Error getting motor attribute '{key}': {type(e).__name__}: {e}") | ||
| except Exception as e: | ||
| logger.error(f"Error processing motor attributes: {type(e).__name__}: {e}") | ||
|
|
||
| try: | ||
| for key in environment_attributes_list: | ||
| try: | ||
| value = getattr(flight_obj.env, key) | ||
| attributes["env"][key] = value | ||
| except AttributeError as e: | ||
| logger.warning(f"Environment attribute '{key}' not found: {e}") | ||
| except Exception as e: | ||
| logger.error(f"Error getting environment attribute '{key}': {type(e).__name__}: {e}") | ||
| except Exception as e: | ||
| logger.error(f"Error processing environment attributes: {type(e).__name__}: {e}") | ||
|
|
||
| return rocketpy_encoder(attributes) | ||
|
|
||
|
|
||
| class RocketPyGZipMiddleware: | ||
|
|
@@ -70,6 +149,7 @@ async def __call__( | |
|
|
||
|
|
||
| class GZipResponder: | ||
| # fork of https://github.com/encode/starlette/blob/master/starlette/middleware/gzip.py | ||
| def __init__( | ||
| self, app: ASGIApp, minimum_size: int, compresslevel: int = 9 | ||
| ) -> None: | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,12 +1,24 @@ | ||||||
| from typing import Optional | ||||||
| from typing import Optional, Any | ||||||
| from datetime import datetime, timedelta | ||||||
| from pydantic import ConfigDict | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Datetime defaults are evaluated at import time; switch to default_factory. Avoid stale defaults in long-lived processes. Apply this diff: -from pydantic import ConfigDict
+from pydantic import ConfigDict, Field
@@
- date: Optional[Any] = datetime.today() + timedelta(days=1)
- local_date: Optional[Any] = datetime.today() + timedelta(days=1)
- datetime_date: Optional[Any] = datetime.today() + timedelta(days=1)
+ date: Optional[Any] = Field(default_factory=lambda: datetime.today() + timedelta(days=1))
+ local_date: Optional[Any] = Field(default_factory=lambda: datetime.today() + timedelta(days=1))
+ datetime_date: Optional[Any] = Field(default_factory=lambda: datetime.today() + timedelta(days=1))Also applies to: 38-40 🤖 Prompt for AI Agents |
||||||
| from src.views.interface import ApiBaseView | ||||||
| from src.models.environment import EnvironmentModel | ||||||
| from src.utils import AnyToPrimitive | ||||||
|
|
||||||
|
|
||||||
| class EnvironmentSimulation(ApiBaseView): | ||||||
| """ | ||||||
| Environment simulation view that handles dynamically encoded RocketPy Environment attributes. | ||||||
|
|
||||||
| Uses the new rocketpy_encoder which may return different attributes based on the | ||||||
| actual RocketPy Environment object. The model allows extra fields to accommodate | ||||||
| any new attributes that might be encoded. | ||||||
| """ | ||||||
|
|
||||||
| model_config = ConfigDict(extra='ignore', arbitrary_types_allowed=True) | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. extra='ignore' contradicts the docstring; use extra='allow'. With extra='ignore', unknown encoded attributes are dropped instead of preserved. Apply this diff: - model_config = ConfigDict(extra='ignore', arbitrary_types_allowed=True)
+ model_config = ConfigDict(extra='allow', arbitrary_types_allowed=True)📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
|
|
||||||
| message: str = "Environment successfully simulated" | ||||||
|
|
||||||
| # Core Environment attributes (always present) | ||||||
| latitude: Optional[float] = None | ||||||
| longitude: Optional[float] = None | ||||||
| elevation: Optional[float] = 1 | ||||||
|
|
@@ -23,30 +35,32 @@ class EnvironmentSimulation(ApiBaseView): | |||||
| initial_hemisphere: Optional[str] = None | ||||||
| initial_ew: Optional[str] = None | ||||||
| max_expected_height: Optional[float] = None | ||||||
| date: Optional[datetime] = datetime.today() + timedelta(days=1) | ||||||
| local_date: Optional[datetime] = datetime.today() + timedelta(days=1) | ||||||
| datetime_date: Optional[datetime] = datetime.today() + timedelta(days=1) | ||||||
| ellipsoid: Optional[AnyToPrimitive] = None | ||||||
| barometric_height: Optional[AnyToPrimitive] = None | ||||||
| barometric_height_ISA: Optional[AnyToPrimitive] = None | ||||||
| pressure: Optional[AnyToPrimitive] = None | ||||||
| pressure_ISA: Optional[AnyToPrimitive] = None | ||||||
| temperature: Optional[AnyToPrimitive] = None | ||||||
| temperature_ISA: Optional[AnyToPrimitive] = None | ||||||
| density: Optional[AnyToPrimitive] = None | ||||||
| speed_of_sound: Optional[AnyToPrimitive] = None | ||||||
| dynamic_viscosity: Optional[AnyToPrimitive] = None | ||||||
| gravity: Optional[AnyToPrimitive] = None | ||||||
| somigliana_gravity: Optional[AnyToPrimitive] = None | ||||||
| wind_speed: Optional[AnyToPrimitive] = None | ||||||
| wind_direction: Optional[AnyToPrimitive] = None | ||||||
| wind_heading: Optional[AnyToPrimitive] = None | ||||||
| wind_velocity_x: Optional[AnyToPrimitive] = None | ||||||
| wind_velocity_y: Optional[AnyToPrimitive] = None | ||||||
| calculate_earth_radius: Optional[AnyToPrimitive] = None | ||||||
| decimal_degrees_to_arc_seconds: Optional[AnyToPrimitive] = None | ||||||
| geodesic_to_utm: Optional[AnyToPrimitive] = None | ||||||
| utm_to_geodesic: Optional[AnyToPrimitive] = None | ||||||
| date: Optional[Any] = datetime.today() + timedelta(days=1) | ||||||
| local_date: Optional[Any] = datetime.today() + timedelta(days=1) | ||||||
| datetime_date: Optional[Any] = datetime.today() + timedelta(days=1) | ||||||
|
|
||||||
| # Function attributes (discretized by rocketpy_encoder, serialized by RocketPyEncoder) | ||||||
| ellipsoid: Optional[Any] = None | ||||||
| barometric_height: Optional[Any] = None | ||||||
| barometric_height_ISA: Optional[Any] = None | ||||||
| pressure: Optional[Any] = None | ||||||
| pressure_ISA: Optional[Any] = None | ||||||
| temperature: Optional[Any] = None | ||||||
| temperature_ISA: Optional[Any] = None | ||||||
| density: Optional[Any] = None | ||||||
| speed_of_sound: Optional[Any] = None | ||||||
| dynamic_viscosity: Optional[Any] = None | ||||||
| gravity: Optional[Any] = None | ||||||
| somigliana_gravity: Optional[Any] = None | ||||||
| wind_speed: Optional[Any] = None | ||||||
| wind_direction: Optional[Any] = None | ||||||
| wind_heading: Optional[Any] = None | ||||||
| wind_velocity_x: Optional[Any] = None | ||||||
| wind_velocity_y: Optional[Any] = None | ||||||
| calculate_earth_radius: Optional[Any] = None | ||||||
| decimal_degrees_to_arc_seconds: Optional[Any] = None | ||||||
| geodesic_to_utm: Optional[Any] = None | ||||||
| utm_to_geodesic: Optional[Any] = None | ||||||
|
|
||||||
|
|
||||||
| class EnvironmentView(EnvironmentModel): | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
collect_simulation_attributesfunction expects 5 parameters but only 2 are provided. According to the function signature in utils.py, it needs flight_obj, flight_simulation_class, rocket_simulation_class, motor_simulation_class, and environment_simulation_class.