8000 Change simulation encoding workaround to official rocketpy encoders by GabrielBarberini · Pull Request #51 · RocketPy-Team/Infinity-API · GitHub
[go: up one dir, main page]
Skip to content

Change simulation encoding workaround to official rocketpy encoders #51

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Change simulation encoding workaround to official rocketpy encoders #51

wants to merge 7 commits into from

Conversation

GabrielBarberini
Copy link
Collaborator
@GabrielBarberini GabrielBarberini commented Jun 1, 2025 •
edited
Loading

Pull Request Overview

This PR shifts the simulation encoding approach to utilize the official RocketPy encoders and a new DiscretizeConfig for improved dynamic attribute support across simulation models. Key changes include:

  • Replacing usage of get_instance_attributes with the new rocketpy_encoder and DiscretizeConfig.
  • Updating simulation view classes (RocketSimulation, MotorSimulation, FlightSimulation, EnvironmentSimulation) to use pydantic’s ConfigDict.
  • Refactoring services to integrate the updated encoding logic for Rocket, Motor, Flight, and Environment simulations.

Refactor

  • Improved how simulation data for environment, flight, motor, and rocket objects is encoded and serialized, enabling more flexible and dynamic handling of simulation attributes.
  • Updated simulation view classes to support dynamic and extensible attributes, allowing for richer and more adaptable simulation outputs.
  • Enhanced attribute typing and documentation for better clarity and flexibility in simulation data representation.

Documentation

  • Added detailed docstrings to clarify the behavior and flexibility of simulation data models.

@GabrielBarberini GabrielBarberini requested review from Copilot and removed request for aasitvora99 June 1, 2025 03:02
@coderabbitai coderabbitai
Copy link
Contributor
coderabbitai bot commented Jun 1, 2025 •
edited
Loading

Walkthrough

This update replaces the use of a generic attribute extraction utility with a new, configurable encoding approach for RocketPy objects. It introduces a DiscretizeConfig class and a rocketpy_encoder function, updating all relevant service and view classes to support dynamic, extensible attributes and flexible serialization using these new tools.

Changes

Files Change Summary
src/services/environment.py
src/services/flight.py
src/services/motor.py
src/services/rocket.py		 Replaced get_instance_attributes with rocketpy_encoder and DiscretizeConfig for object serialization before simulation instantiation.
src/utils.py Added DiscretizeConfig class and rocketpy_encoder function; removed to_python_primitive and related type alias.
src/views/environment.py
src/views/flight.py
src/views/motor.py
src/views/rocket.py		 Refactored simulation view classes to support dynamic attributes, allow extra fields, and updated type hints for flexibility.

Sequence Diagram(s)

sequenceDiagram
    participant ServiceClass as Service (e.g., RocketService)
    participant RocketPyObj as RocketPy Object
    participant Encoder as rocketpy_encoder
    participant Config as DiscretizeConfig
    participant Simulation as SimulationView

    ServiceClass->>RocketPyObj: Access instance (e.g., self.rocket)
    ServiceClass->>Config: Get config (e.g., DiscretizeConfig.for_rocket())
    ServiceClass->>Encoder: rocketpy_encoder(RocketPyObj, Config)
    Encoder->>RocketPyObj: Discretize and encode attributes
    Encoder-->>ServiceClass: Encoded dict
    ServiceClass->>Simulation: SimulationView(**encoded_dict)
Loading

Suggested reviewers

  • luimot
  • phmbressan
  • aasitvora99
  • Gui-FernandesBR

Poem

In the garden of code where rockets fly high,
We swapped old extractors for encoders—oh my!
With configs that discretize, and views that expand,
Our models now flex at the encoder’s command.
🐇✨
Dynamic as spring, our API’s in bloom—
RocketPy rabbits, make plenty of room!

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9339a43 and 8450082.

📒 Files selected for processing (5)
  • src/utils.py (2 hunks)
  • src/views/environment.py (2 hunks)
  • src/views/flight.py (1 hunks)
  • src/views/motor.py (1 hunks)
  • src/views/rocket.py (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • src/views/environment.py
  • src/views/flight.py
  • src/views/motor.py
  • src/views/rocket.py
🔇 Additional comments (4)
src/utils.py (4)

3-5: LGTM: Imports and logger setup are appropriate.

The new imports support the encoding functionality well:

  • logging and json for the encoding process
  • copy for deep copying objects to avoid mutation
  • RocketPy imports for the official encoder integration

The logger setup follows sta 8000 ndard Python practices.

Also applies to: 7-7, 9-10, 14-14

17-46: Well-designed configuration class with sensible defaults.

The DiscretizeConfig class provides a clean, extensible way to configure discretization parameters for different RocketPy object types. The factory methods offer object-specific presets that appear well-suited for their respective domains:

  • Environment: Wide bounds (0-50000) for atmospheric data
  • Motor: Narrow bounds (0-10) with higher sampling for precision
  • Rocket: Small bounds (0-1) for mass ratios/coefficients
  • Flight: Moderate bounds (0-30) for time-based data

The class design follows good practices with clear documentation and reasonable defaults.

48-110: Excellent implementation addressing previous mutation concerns.

The rocketpy_encoder function successfully addresses the critical object mutation issue identified in previous reviews by using copy.deepcopy() at line 65. The implementation demonstrates several strengths:

  • Safe object handling: Creates a deep copy to prevent side effects
  • Robust error handling: Graceful exception handling throughout
  • Official encoder integration: Properly uses RocketPyEncoder with appropriate options
  • Fallback mechanism: Provides alternative encoding when primary method fails

The function correctly discretizes Function attributes on the copy while preserving the original object's state, which is essential for object reusability across multiple simulations.

However, note that the fallback mechanism at lines 107 converts all values to strings, which may lose type information as mentioned in previous reviews. This is an acceptable trade-off for robustness since it only activates when the primary encoder fails.

138-138: Good practice: Attribution for forked code.

Adding the source attribution comment for the forked starlette middleware code follows good open-source practices and licensing compliance.

✨ Finishing Touches
  • 📝 Generate Docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copilot

This comment was marked as duplicate.

Sign in to view

coderabbitai[bot]

This comment was marked as outdated.

Sign in to view

@GabrielBarberini GabrielBarberini requested a review from luimot June 1, 2025 03:11
@GabrielBarberini GabrielBarberini requested a review from Copilot June 1, 2025 03:13
Copilot

This comment was marked as duplicate.

Sign in to view

GabrielBarberini and others added 3 commits June 1, 2025 00:14
@GabrielBarberini
removes remaining redundant init
7c7f805
@GabrielBarberini @Copilot
Update src/utils.py
6ecc358 
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@GabrielBarberini
update docs
5eab472
@GabrielBarberini GabrielBarberini requested a review from Copilot June 1, 2025 03:23
Copilot

This comment was marked as outdated.

Sign in to view

@GabrielBarberini GabrielBarberini requested a review from Copilot June 1, 2025 03:32
Copilot
Copilot AI reviewed Jun 1, 2025
View reviewed changes
Copy link
@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR migrates simulation encoding to use the official RocketPy encoders and introduces a new DiscretizeConfig for improved dynamic attribute support across simulation models.

  • Replaces get_instance_attributes with rocketpy_encoder and config objects in service files.
  • Updates simulation view classes (Rocket, Motor, Flight, Environment) to use pydantic’s ConfigDict and dynamic extra attributes.
  • Refactors the utils module by adding DiscretizeConfig and enhancing the rocketpy_encoder logic to handle callable functions.

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/views/rocket.py Updated RocketSimulation with new field types and removal of default for coordinate_system_orientation.
src/views/motor.py Updated MotorSimulation to extend ApiBaseView with new dynamic attributes and altered defaults.
src/views/flight.py Revised FlightSimulation to integrate new encoding logic and dynamic flight attribute support.
src/views/environment.py Modified EnvironmentSimulation to include updated function attribute fields.
src/utils.py Introduced DiscretizeConfig and enhanced rocketpy_encoder to work with official RocketPyEncoder.
src/services/*.py Updated service methods to replace get_instance_attributes with rocketpy_encoder and proper config usage.
Comments suppressed due to low confidence (2)

src/views/rocket.py:29

  • Removing the default value ('tail_to_nose') for coordinate_system_orientation in RocketSimulation may break downstream behavior that expects this default; please confirm if this change is intentional or reinstate the default.
coordinate_system_orientation: Optional[str] = None

src/views/motor.py:35

  • The default value for coordinate_system_orientation has been removed (previously 'nozzle_to_combustion_chamber'); confirm that this modification is intended as it may affect components relying on that default.
coordinate_system_orientation: Optional[str] = None

@GabrielBarberini
Copy link
Collaborator Author

@coderabbitai review

@coderabbitai coderabbitai
Copy link
Contributor
coderabbitai bot commented Jun 1, 2025
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

Copilot code review Copilot Copilot left review comments

@aasitvora99 aasitvora99 Awaiting requested review from aasitvora99 aasitvora99 is a code owner

@Gui-FernandesBR Gui-FernandesBR Awaiting requested review from Gui-FernandesBR

@phmbressan phmbressan Awaiting requested review from phmbressan

@luimot luimot Awaiting requested review from luimot

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@GabrielBarberini
0