Skip to content
This repository has been archived by the owner on Oct 30, 2022. It is now read-only.

Issues #18, #72, #73, #74, #69: team-07 #110

Open
wants to merge 22 commits into
base: master
Choose a base branch
from

Conversation

Jasn6z
Copy link

@Jasn6z Jasn6z commented Aug 28, 2022

Pull request type

Please check the type of change your PR introduces:

  • Code base additions (bugfix, features)
  • Code maintenance (refactoring, formatting, renaming, tests)
  • ReadMe, Docs and GitHub maintenance
  • Other (please describe):

Pull request checklist

Please check if your PR fulfills the following requirements, depending on the type of PR:

  • ReadMe, Docs and GitHub maintenance:

    • Spelling has been verified
    • Code docs are working correctly
  • Code base maintenance (refactoring, formatting, renaming):

    • Docs have been reviewed and added / updated if needed
    • Lint (black rocketpy) has passed locally and any fixes were made
    • All tests (pytest --runslow) have passed locally
  • Code base additions (for bug fixes / features):

    • Tests for the changes have been added
    • Docs have been reviewed and added / updated if needed
    • Lint (black rocketpy) has passed locally and any fixes were made
    • All tests (pytest --runslow) have passed locally

What is the current behavior?

Enter text here...

What is the new behavior?

Enter text here...

Does this introduce a breaking change?

  • Yes
  • No

Other information

Enter text here...

@Jasn6z Jasn6z requested a review from giovaniceotto as a code owner August 28, 2022 23:42
@Gui-FernandesBR Gui-FernandesBR changed the title #18 #72 #73 #74 #69 07 Issues #18, #72, #73, #74, #69: team-07 Aug 29, 2022
@Gui-FernandesBR Gui-FernandesBR removed the request for review from giovaniceotto August 29, 2022 05:33
@Gui-FernandesBR Gui-FernandesBR linked an issue Aug 29, 2022 that may be closed by this pull request
@FranzYuri
Copy link

FranzYuri commented Sep 2, 2022

Hi @Jasn6z! Great work here! This new class Analysis is incredibly organized and have some very interesting methods.
A few questions though:
First, at snatchforce_calculator, what is the formula you used, and are you sure using the velocity of impact was the way to go? Also, were you indeed intending to have as output a Function object? Was the idea to have a function of altitude vs snatch force?
Secondly, I could not find where you got the formula for chute_radius_finder at line 101 in Analysis.py. Are you sure this formula could be used for toroidal parachutes?

@FranzYuri FranzYuri closed this Sep 2, 2022
@Jasn6z
Copy link
Author

Jasn6z commented Sep 2, 2022 via email

@FranzYuri FranzYuri added 25 points Medium challenges! 50 points Hard challengs! and removed 25 points Medium challenges! labels Sep 3, 2022
@FranzYuri FranzYuri added penalty Solutions poorly documented or badly coded 0 points Not receiving points for this PR labels Sep 3, 2022
@FranzYuri
Copy link

Hi @Jasn6z ! Again, great work here! I am giving you 50 points for the issues 18 and 69 (both of medium difficulty) and also 50 for the issue 73. Now, about the issues 72 and 74: at 72 the formula you used to find the diameter through the area is wrong, as it should be D²*pi/4=Area. Indeed, searching closely I found it was also wrong at Fruity Chutes, but I still will have to give you a penalty, resulting in 40 points instead of 50.
At 74, I read the sources you gave me and, looking at the calculations presented at https://apps.dtic.mil/sti/citations/ADA180901, you forgot the shock factor (x1), that should be multiplied by the drag force of the parachute fully open in its final velocity (you can read further at pages 10 and 11).
image
As shown here, we cannot ignore this coefficient or consider it to be 1, as it affects greatly the final result. Therefore, I unfortunately will not give you points for this task.
With all that, you got 140 points for this pull request! A gigantic congratulations for you and your team!

If you have doubts or disagree with my assesment fell free to contact us in any way you like (here, on discord, linkedin or so on).

@Gui-FernandesBR
Copy link
Member

Gui-FernandesBR commented Sep 3, 2022

Regarding issues 18 and 69, May recieve an extra 20% points since it was the onlly team solving this challege

@Gui-FernandesBR Gui-FernandesBR added the extra_points Exceptionally ingenious solutions label Sep 3, 2022
@Gui-FernandesBR Gui-FernandesBR removed the request for review from MateusStano September 3, 2022 18:45
@FranzYuri
Copy link

HI again! Unfortunately, after closer examination we concluded that it would not be fair to give your team points for the issue 72, as it was not a complete solution for the challenge. For designing a parachute, it was expect the code to find at least the height and radius for an ellipsoidal parachute, internal and external radii for a toroidal, allow user to choose the area of the vent and so on. The current implementation presented on the method chute_radius_finder does not help on any of that and also fails to find the right D0 of the parachute, as I said earlier.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
0 points Not receiving points for this PR 25 points Medium challenges! 50 points Hard challengs! extra_points Exceptionally ingenious solutions penalty Solutions poorly documented or badly coded team-07
Projects
6 participants