Skip to content
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

BUG: Allow multiple sets of stochastic fins #737

Merged
merged 1 commit into from
Nov 24, 2024

Conversation

WilliamArmst
Copy link
Contributor

@WilliamArmst WilliamArmst commented Nov 24, 2024

Pull request type

Bugfix

Checklist

  • Lint (black rocketpy/ tests/) has passed locally
  • All tests (pytest tests -m slow --runslow) have passed locally
  • CHANGELOG.md has been updated (if relevant)

Current behavior

If there is a rocket with 2 sets of fins and you try to make them stochastic, the function that is fixed in the code will throw because it only checks the first set of fins. This makes it impossible to add 2 sets of fins.

Here is a sample program if you wish to confirm:

from rocketpy import TrapezoidalFins, Rocket, StochasticTrapezoidalFins, StochasticRocket

# apply percent standard deviation to components
percentStdDeviation = 0.1

testRocket = Rocket( # All of the data is incorrect
    radius=0.1,
    mass=1,
    inertia=(6.321, 6.321, 0.034),
    power_off_drag= "powerOffDragCurve.csv",
    power_on_drag= "powerOnDragCurve.csv",
    center_of_mass_without_motor=0.5,
    coordinate_system_orientation="tail_to_nose",
)
fin_set_1 = testRocket.add_trapezoidal_fins(
    n=4,
    root_chord=0.203,
    tip_chord=0.0381,
    span=0.102,
    sweep_length=0.21,
    position=-0.015,
    cant_angle=0.5,
    airfoil=("airfoil_example.csv","radians"), # insert a .csv file for the side profile of the fin
)
fin_set_2 = testRocket.add_trapezoidal_fins(
    n = 4,
    root_chord=0.203,
    tip_chord=0.0381,
    span=0.102,
    sweep_length=0.21,
    position=-1.945,
    cant_angle=0.5,
    airfoil=("airfoil_example.csv","radians"), # insert a .csv file for the side profile of the fin
)
fin_set_3 = TrapezoidalFins(
    n = 4,
    root_chord=0.203,
    tip_chord=0.0381,
    span=0.102,
    sweep_length=0.21,
    cant_angle=0.5,
    rocket_radius=0.1,
    airfoil=("airfoil_example.csv","radians"), # insert a .csv file for the side profile of the fin
)

stochastic_fin_set_1_object = StochasticTrapezoidalFins(
    trapezoidal_fins=fin_set_1,
    root_chord=percentStdDeviation * fin_set_1.root_chord,
    tip_chord=percentStdDeviation * fin_set_1.tip_chord,
    span=percentStdDeviation * fin_set_1.span,
    sweep_length=percentStdDeviation * fin_set_1.sweep_length,
    cant_angle=0.01, # +- 0.01deg
)
stochastic_fin_set_2_object = StochasticTrapezoidalFins(
    trapezoidal_fins=fin_set_2,
    root_chord=percentStdDeviation * fin_set_2.root_chord,
    tip_chord=percentStdDeviation * fin_set_2.tip_chord,
    span=percentStdDeviation * fin_set_2.span,
    sweep_length=percentStdDeviation * fin_set_2.sweep_length,
    cant_angle=0.01, # +- 0.01deg
)

stochastic_testRocket = StochasticRocket(
    rocket=testRocket,
    radius=0.1 * testRocket.radius,
    mass=0.1 * testRocket.mass,
    center_of_mass_without_motor=0.1 * testRocket.center_of_mass_without_motor,
)

stochastic_fin_set_sustainer_Event1 = stochastic_testRocket.add_trapezoidal_fins(
   fins=stochastic_fin_set_1_object,
   position=0.01, # +-1cm
)

stochastic_fin_set_booster_Event1 = stochastic_testRocket.add_trapezoidal_fins(
    fins=stochastic_fin_set_2_object,
    position=0.01, # +-1cm
)

testRocket.draw()

I also tested adding a stochastic fin that actually wasn't a part of the rocket, and it still catches that error (replace one of the trapezoidal_fins= with fin_set_3 to see this work)

New behavior

Behaves as expected.

Breaking change

  • Yes
  • No

Additional information

I thought I figured out the git stuff but ig I didn't. Oh well. I am allowed to not know how to use git because I am fixing your bugs.

@WilliamArmst WilliamArmst changed the title Allow multiple sets of stochastic fins BUG: Allow multiple sets of stochastic fins Nov 24, 2024
@WilliamArmst WilliamArmst marked this pull request as ready for review November 24, 2024 03:12
@WilliamArmst WilliamArmst requested a review from a team as a code owner November 24, 2024 03:12
Copy link

codecov bot commented Nov 24, 2024

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 75.99%. Comparing base (1f3eefc) to head (8a06c01).

Files with missing lines Patch % Lines
rocketpy/stochastic/stochastic_rocket.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #737      +/-   ##
===========================================
+ Coverage    75.97%   75.99%   +0.01%     
===========================================
  Files           95       95              
  Lines        11027    11025       -2     
===========================================
  Hits          8378     8378              
+ Misses        2649     2647       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@Gui-FernandesBR Gui-FernandesBR added Bug Something isn't working Monte Carlo Monte Carlo and related contents labels Nov 24, 2024
@Gui-FernandesBR
Copy link
Member

@WilliamArmst regarding git, we are working on an improvement to our development documentation, I think that should help you to understand what is happening.

This is what I recommend you to do:

git checkout origin develop
git pull
git checkout -   # this goes to your previous branch
git rebase -i develop  # this will rebase your branch on top of develop, allowing you to decide what to do with each commit

This will open an editor, which could be vim or nano, for example.
Delete all the lines that contains commits you do not want to include in this PR. Simply let the last 2 commits, you can keep the "pick" option by the begining of each line.
Save the operation and see the

git log --graph --oneline

You should expect something like this:
image

If everything is correct, try to push again:

git push

or, as I like to do:

git push -f

@WilliamArmst
Copy link
Contributor Author

Ok so I did not do it right.

@Gui-FernandesBR
Copy link
Member

Ok so I did not do it right.

Don't worry, we can get you from here. Not a problem.

Dealing with rebase isn't so trivial.

Next branch you open, try to git pull before creating the new branch, that might help you.

If you plan to open more PRs, please let me know so I grant you permission to create branches directly in our repos instead of your fork, that could make your life easier.

@WilliamArmst
Copy link
Contributor Author

I think I got it

@WilliamArmst
Copy link
Contributor Author

WilliamArmst commented Nov 24, 2024

I think it might be because I only forked the master branch, but I now have the develop one too. I should be all good going forward. Thanks for your help!

Copy link
Member

@Gui-FernandesBR Gui-FernandesBR left a comment

Choose a reason for hiding this comment

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

Tests are passing locally, including slow tests.

This time I won't wait for regression tests, let's keep moving.

Thx again for another problem solved, @WilliamArmst !

@Gui-FernandesBR Gui-FernandesBR merged commit 0fe3d8d into RocketPy-Team:develop Nov 24, 2024
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Monte Carlo Monte Carlo and related contents
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

2 participants