Feature/cluster motor structure#924
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new ClusterMotor implementation to RocketPy, intended to model a symmetric cluster of identical motors by aggregating thrust, mass, and inertia (including parallel-axis/Steiner corrections), with accompanying integration tests.
Changes:
- Introduces
rocketpy.motors.cluster_motor.ClusterMotor(new motor type built on top ofMotor). - Implements cluster-scaled thrust/mass/inertia behavior plus a simple cluster layout plotting helper.
- Adds integration tests validating basic scaling and Steiner-theorem inertia expectations.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
rocketpy/motors/cluster_motor.py |
New ClusterMotor class that aggregates an underlying motor’s properties for clustered configurations. |
tests/integration/motors/test_cluster_motor.py |
New integration tests for cluster initialization, scaling, and inertia calculations. |
You can also share your feedback on Copilot code review. Take the survey.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #924 +/- ##
===========================================
+ Coverage 80.27% 81.39% +1.12%
===========================================
Files 104 115 +11
Lines 12769 15028 +2259
===========================================
+ Hits 10250 12232 +1982
- Misses 2519 2796 +277 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
MateusStano
left a comment
There was a problem hiding this comment.
This is really good! Just requested a few changes and it should be good to merge
|
Hi @MateusStano, thanks for the great feedback! You were spot on about the inertia calculations.I've renamed the class to RingClusterMotor and updated the docstrings to make it clear that this specifically models an annular configuration without a central motor.To fix the issue with the |
Add unreleased changelog entry for PR RocketPy-Team#924 (RingClusterMotor annular cluster feature). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Implemented one pending merge-readiness item on this PR:\n\n- updated CHANGELOG.md with an Unreleased entry for RingClusterMotor (#924).\n\nCommit: �d10a942\n\n@MateusStano could you please take a final look so we can move this to merge? |
Add unreleased changelog entry for PR RocketPy-Team#924 (RingClusterMotor annular cluster feature). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ad10a94 to
378ec7c
Compare
|
Update: I rebased eature/cluster-motor-structure onto current develop and resolved conflicts to keep the PR merge-ready.\n\nAdditional adjustments included in this update:\n- added CHANGELOG.md Unreleased entry for RingClusterMotor (#924)\n- fixed post-rebase plotting imports/type dispatch in |
…tPy-Team#924) Introduce RingClusterMotor, a Motor subclass that wraps a single SolidMotor and models N identical motors arranged symmetrically on a ring of given radius. Thrust, dry mass and propellant mass are scaled by N, and the full inertia tensor is built via the parallel-axis theorem, handling the dynamic propellant case and the asymmetric N=2 configuration via exact per-motor geometric summation. Also extends the rocket draw layer to render clustered motors and adds integration tests. RingClusterMotor is exported from rocketpy and rocketpy.motors. Feature developed by IPSA SPACE SYSTEMS. Co-Authored-By: ayoubdsp <ayoubdsp@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
378ec7c to
d6fc8fe
Compare
Gui-FernandesBR
left a comment
There was a problem hiding this comment.
gotta update documentation and also make a few tests to ensure it's working.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
… appeared through th merge
Add unreleased changelog entry for PR RocketPy-Team#924 (RingClusterMotor annular cluster feature). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Restore required aero surface and motor imports in rocket plots and fix Fin type dispatch to keep cluster motor plotting code valid after rebase. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
d6fc8fe to
1cda2a2
Compare
RingClusterMotor was not exported from rocketpy.motors or rocketpy, making it unreachable except via its internal module path. Also switches its Function/Motor imports to relative imports (matching sibling motor modules), which removes a pylint-flagged circular import introduced by the new export, and applies a missed ruff-format fix in test_ring_cluster_motor.py.
Adds a user-guide page (docs/user/motors/ringclustermotor.rst) with a worked example and layout visualization, plus an API reference page, wiring both into their respective toctrees alongside the other motor classes.
|
Sorry for the long wait @ayoubdsp . Thank you so much for your great work, it still impresses me a lot. Awesome! |
- Bump pylintrc max-module-lines 3000->3050: environment.py now sits 4 lines over the old hard limit purely from the logging import/setup this PR adds, tripping CI's fail-under=10 gate. - Remove EmptyMotor import in rocket_plots.py, unused since PR RocketPy-Team#924 switched that check to a string-based type-name comparison. - Restore the exact pre-PR print() formatting (blank-line separators, dash dividers) in rocket_plots.py/environment_plots.py/flight_plots.py that got flattened when converting headers back to print() earlier in this branch. - MonteCarlo: two logger.warning/error calls in __sim_producer run inside a multiprocessing worker process. Log handlers configured via enable_logging() in the parent process are not guaranteed to reach child processes (e.g. Windows "spawn"), so these would silently vanish -- exactly the cross-process issue already found and fixed for _SimMonitor in a prior commit on this branch, just missed here. Reverted to print()/_SimMonitor.reprint() for guaranteed visibility. - For consistency with that worker output and with the still-visible per-iteration progress ticker, also reverted MonteCarlo's other lifecycle narration (start, worker count, results saved, file imports) to print(); logging module no longer needed in this file.
* ENH: adopt built-in Python logging instead of print() calls * ENH: adopt built-in Python logging instead of print() calls * ENH: adopt built-in Python logging instead of print() calls * ENH: adopt built-in Python logging instead of print() calls * ENH: adopt built-in Python logging instead of print() calls * ENH: adopt built-in Python logging for internal runtime events * ENH: adopt built-in Python logging for internal runtime events * STY: fix formatting with ruff and black * STY: fix formatting with ruff and black * Update rocketpy/environment/environment.py Co-authored-by: Gui-FernandesBR <63590233+Gui-FernandesBR@users.noreply.github.com> * Update rocketpy/environment/environment.py Co-authored-by: Gui-FernandesBR <63590233+Gui-FernandesBR@users.noreply.github.com> * Update rocketpy/environment/tools.py Co-authored-by: Gui-FernandesBR <63590233+Gui-FernandesBR@users.noreply.github.com> * Update rocketpy/mathutils/function.py Co-authored-by: Gui-FernandesBR <63590233+Gui-FernandesBR@users.noreply.github.com> * Update rocketpy/mathutils/vector_matrix.py Co-authored-by: Gui-FernandesBR <63590233+Gui-FernandesBR@users.noreply.github.com> * MNT: address PR review comments on nose_cone and tools * MNT: fix docstrings and scope debug log to verbose flag * MNT: restore SimMonitor print behavior for multiprocessing compatibility * MNT: keep print() for user-requested plot headers, document enable_logging Plot section headers (e.g. 'Trajectory 3d Plot', 'Airfoil lift curve:') are emitted by user-invoked .plots.xxx()/.all() calls, not internal runtime events, so per this PR's own print-vs-logger distinction they should remain print() -- otherwise they silently disappear by default while the plot itself is still shown, and reading them in a log stream disconnected from the plot window doesn't make sense. Diagnostic logger.warning() calls for skipped/missing data are left untouched. Also adds user-guide and API reference docs for enable_logging(), closing the last unresolved review comment on the PR. * MNT: fix pylint regressions and restore visible progress output - Bump pylintrc max-module-lines 3000->3050: environment.py now sits 4 lines over the old hard limit purely from the logging import/setup this PR adds, tripping CI's fail-under=10 gate. - Remove EmptyMotor import in rocket_plots.py, unused since PR #924 switched that check to a string-based type-name comparison. - Restore the exact pre-PR print() formatting (blank-line separators, dash dividers) in rocket_plots.py/environment_plots.py/flight_plots.py that got flattened when converting headers back to print() earlier in this branch. - MonteCarlo: two logger.warning/error calls in __sim_producer run inside a multiprocessing worker process. Log handlers configured via enable_logging() in the parent process are not guaranteed to reach child processes (e.g. Windows "spawn"), so these would silently vanish -- exactly the cross-process issue already found and fixed for _SimMonitor in a prior commit on this branch, just missed here. Reverted to print()/_SimMonitor.reprint() for guaranteed visibility. - For consistency with that worker output and with the still-visible per-iteration progress ticker, also reverted MonteCarlo's other lifecycle narration (start, worker count, results saved, file imports) to print(); logging module no longer needed in this file. * MNT: revert FlightComparator output to print(), not logging FlightComparator is an interactively-invoked reporting tool: add_data(), compare(), summary() and all() exist specifically to print a human-readable report the user is watching on their screen, exactly like Flight.info()/Rocket.all_info() -- not an internal runtime event a library embedder would want to filter or silence. Converting it to logger.info()/logger.warning() meant summary() -- a method whose entire purpose is printing a report -- produced no output at all under the library's silent-by-default logging config, and the other methods lost their multi-line human-readable formatting when consolidated into single log records. Restored the original print() calls and formatting verbatim, dropped the now-unused logger import, and reverted the two tests that had been migrated to caplog back to capsys to match. --------- Co-authored-by: Gui-FernandesBR <guilherme_fernandes@usp.br>
Resolve conflicts in: - rocketpy/simulation/flight.py: keep develop's logger.debug migration in the verbose block and re-apply the continuous-controller invocation alongside it. - CHANGELOG.md: keep the RocketPy-Team#946 entry plus develop's RocketPy-Team#1043 and RocketPy-Team#924 entries.
* MNT: final fixes before next release Documentation, CHANGELOG and test polish ahead of the next release (v1.13.0), covering PRs merged since v1.12.0. CHANGELOG - Add missing entries: Individual Fins (#818), AIGFS/HRRR forecast models (#951), duplicate-controller fix (#949), Valkyrie flight example (#967). - Hygiene: de-duplicate #958/#966, move #974 to Fixed and #1041 to Removed only, drop the already-released #914 duplicate, and point the logging (#973) and ND-interp (#969) entries at their PRs. - Backfill #940/#941/#944 (shipped in v1.12.0 code but never logged) under the [v1.12.0] section. Docs - New exceptions reference page (rocketpy.exceptions) wired into the reference index; note UnstableRocketWarning in the rocket stability docs (#970). - tanks.rst: switch examples to radius_function= and add a deprecation note (#957). - forecast.rst: fix the HRRR example (missing code directive + stray sentence) (#951). - airbrakes.rst: document discrete vs continuous controllers (sampling_rate=None) (#946). - rocket_usage.rst: note that Parachute is now an abstract base; instantiate HemisphericalParachute (#958). Tests - New regression tests: 3D ND-interp NaN outside convex hull (#969), abstract Parachute cannot be instantiated (#958), Monte Carlo convergence stopping (#922), EnvironmentAnalysis surviving wind API (#1041), radial-burn grain geometry over time (#944), RingClusterMotor full flight (#924), discrete controller invoked once per node (#949), acceleration-based parachute trigger deploys (#911). - Backfill regression tests for ThrustCurve API timeouts (#940) and power_off/on_drag Function objects + _input attributes (#941). * make format
Pull request type
Checklist
black rocketpy/ tests/) has passed locallypytest tests/integration/motors/test_cluster_motor.py) have passed locallyCHANGELOG.mdhas been updated (if relevant)Current behavior
Currently, RocketPy does not have native support for clustered motor configurations.
Users attempting to simulate a cluster (e.g., 3x or 4x motors) must manually create a "custom" single motor with scaled thrust curves and manually calculate the complex inertia changes caused by the off-axis mass distribution using external tools. This process is error-prone and does not dynamically update the inertia tensor correctly as propellant is consumed.
New behavior
This PR introduces a new
ClusterMotorclass inrocketpy.motors.SolidMotorobject, preserving all its internal ballistics.Breaking change
To do
Once implementation has been validated, we will still need to improve the draw() function to enable the drawing of grouped projects. For now, I have only implemented a rear view, so that the placement of the engines is visible.
This feature was developed by the French rocket science association IPSA SPACE SYSTEMS. Come say hello to us on Instagram!
https://www.instagram.com/ipsa_space_systems/