-
-
Notifications
You must be signed in to change notification settings - Fork 235
TST: Add acceptance tests for 3DOF flight simulation based on Bella Lui rocket #914
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
Conversation
|
@copilot Please check if the fixtures from flight_fixtures.py can be reused for these tests or not. If yes then reuse them here. If not then add new fixtures to the flight_fixtures.py file. |
Done! I've refactored the fixtures as requested: Changes made (commit c6cbc90):
Results:
|
|
@copilot Pointmassrocket and pointmassmotor are avialable in the base branch as well as the branch you have created. They should be utlised here for 3 dof flight accpetance tests. As they form the baseline of flight 3 dof sims. The flight fixtures already have acceptance forms of both the pointmassmotor and pointmassrocket. so why are they being called in the test_3dof_flight.py. Seems redundant. Analyze this line of thought in a critic sense and make changes. Similary why implement try and except for both the classes when they are available within the code base. check the codebase thoroughly please. |
You're absolutely right! I've analyzed the codebase thoroughly and made the necessary changes (commit 7eab61d): Issues found and fixed:
Changes made:
The fixtures properly utilize PointMassMotor and PointMassRocket as the baseline for 3 DOF flight simulations as intended. |
aZira371
left a comment
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.
@copilot Please review the comments and implement requested changes, if applicable.
All requested changes have been implemented in commit 85a0eff:
The tests now provide realistic acceptance testing using the Bella Lui rocket as a baseline, with all parameters properly documented and all magic numbers replaced with meaningful constants. |
|
@copilot The tolerances related to weathercocking are not realistic. The changes in flight trajectory due to weathercoking are hard to quantify on, instead of checking for tolerances there, we should ideally track if the properties are changing or not. The tests which check the effect for weathercocking should probably look for evaluating whether the physics implementation is happening or not. |
Agreed! I've refactored the weathercocking tests to verify physics implementation rather than check specific tolerances (commit 0f765af). Changes:
Tests now verify that weathercocking physics is being applied (trajectory changes occur) without asserting specific magnitude thresholds that are hard to quantify. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #914 +/- ##
===========================================
+ Coverage 80.27% 81.10% +0.83%
===========================================
Files 104 107 +3
Lines 12769 13833 +1064
===========================================
+ Hits 10250 11219 +969
- Misses 2519 2614 +95 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: aZira371 <[email protected]>
… than specific tolerances Co-authored-by: aZira371 <[email protected]>
0f765af to
e48a5fe
Compare
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.
Pull request overview
This PR adds comprehensive acceptance tests for the 3DOF (3 Degrees of Freedom) flight simulation mode in RocketPy. The tests validate trajectory physics, energy conservation, weathercocking behavior, mass variation, and reproducibility using realistic Bella Lui rocket parameters.
- Adds 12 new acceptance tests covering key flight physics and edge cases for 3DOF simulations
- Introduces test fixtures for point mass motors and rockets based on AeroTech K828FJ motor specifications
- Establishes baseline acceptance criteria with calibrated tolerance ranges based on actual Bella Lui flight data (~459m apogee)
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| tests/fixtures/flight/flight_fixtures.py | Adds 6 new fixtures for 3DOF testing: motor and rocket configurations based on Bella Lui parameters, plus flight fixtures with and without weathercocking. Also defines module-level launch parameter constants. |
| tests/acceptance/test_3dof_flight.py | New test suite with 12 acceptance tests validating trajectory, energy conservation, weathercocking physics, mass variation, thrust profiles, quaternion evolution, and reproducibility for 3DOF flight simulations. |
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
…fixtures Co-authored-by: aZira371 <[email protected]>
Gui-FernandesBR
left a comment
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.
@aZira371 good work!
Please address all the comments created by copilot so we can proceed with merging this PR
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.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: aZira371 <[email protected]>
- MNT: added required line to changelog.md
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.
@Gui-FernandesBR Changes made according to copilot review with my suggestions as response. PR should be good to go. Change log has been updated as well. Running one last commit to close everything.
…effects Co-authored-by: aZira371 <[email protected]>
Pull request type
Checklist
black rocketpy/ tests/) has passed locallypytest tests -m slow --runslow) have passed locallyCurrent behavior
No acceptance tests exist for 3DOF flight simulation feature. Integration tests exist but lack end-to-end validation with realistic scenarios.
New behavior
Adds comprehensive acceptance test suite (
tests/acceptance/test_3dof_flight.py) with 12 tests covering:Tests use realistic Bella Lui rocket parameters for acceptance testing baseline:
Tolerance ranges calibrated to Bella Lui performance:
Named constants throughout: All magic numbers extracted to meaningful constants with clear documentation:
Physics-based weathercocking verification: Rather than checking specific tolerance thresholds (which are hard to quantify for weathercocking effects), tests verify that:
Tolerance-based assertions: All non-weathercocking comparisons use explicit named tolerances with clear error messages showing expected vs actual values with tolerance context.
Critical bug fixes addressed during review:
set()uniqueness checks with range-based comparisons to avoid floating-point precision issuesFixture Organization: All 3DOF-specific fixtures centralized in
tests/fixtures/flight/flight_fixtures.py:acceptance_point_mass_motor: Motor based on K828FJ specifications with Bella Lui parametersacceptance_point_mass_rocket: Rocket based on Bella Lui parameters (corrected radius)flight_3dof_no_weathercock: 3DOF flight without weathercocking (weathercock_coeff=0.0)flight_3dof_with_weathercock: 3DOF flight with weathercocking enabled (weathercock_coeff=1.0)The fixtures reuse existing
example_spaceport_envfromenvironment_fixtures.py. All fixtures properly import and utilize PointMassMotor and PointMassRocket from the codebase.Breaking change
Additional information
Tests are fully executable using the existing 3DOF implementation in the codebase (
rocketpy/motors/point_mass_motor.pyandrocketpy/rocket/point_mass_rocket.py). All existing acceptance tests (bella_lui, ndrt_2020, prometheus) pass unchanged. CodeQL: 0 alerts.Fixtures follow the project's pattern of centralizing test fixtures in
tests/fixtures/directory. Imports are properly structured at the top of files following Python conventions. All constants are appropriately named for their specific purpose with zero duplication across files.Tolerance ranges are based on actual Bella Lui flight data analysis, ensuring realistic expectations while allowing for the point mass approximation and atmospheric variations inherent in 3DOF simulations. Weathercocking tests verify physics implementation by checking for trajectory changes rather than asserting specific quantitative thresholds that are difficult to determine realistically. Quaternion tests account for passive aerodynamic effects that occur even without active weathercocking model.
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.