ENH: Free-Form Fins #694
ENH: Free-Form Fins #694
Conversation
MateusStano
added
Enhancement
There was a problem hiding this comment.
Really nice feature, this is quite important for us.
1 - a good test would be to create a FreeFormFins set with the same size as a trapezoidal fins so we can validate that the center of pressure is the same when using both Trapezoidal and FreeForm fins
2 - can you provide a minimum example of using the class? It is still a bit abstract to me; i.e. I can`t visualize how to use this new feature.
Well done!
| def draw(self): | ||
| """Draw the fin shape along with some important information, including | ||
| the center line, the quarter line and the center of pressure position. | ||
|
|
||
| Returns | ||
| ------- | ||
| None | ||
| """ |
There was a problem hiding this comment.
The draw method is not closing the fin draw when the last point is different than the initial point, as shown in the figure below
There was a problem hiding this comment.
how are we going to close it? linearly? we should provide an example (in a future PR maybe) showing case when the fin is set at a non-planar surface. for example when the fins are placed on the tail/transition
There was a problem hiding this comment.
Well, I assumed that the Fin is defined by a closed polygon. Indeed, the documentation states that "the last point should be the first one." So, if the user forgot to make sure the last point is the first one, we either should raise an error or just close it ourselves.
There was a problem hiding this comment.
The model assumes that it is closed linearly. I will add the last line in the plot as well
| down = False | ||
| for i in range(1, len(shape_points)): | ||
| if shape_points[i][1] > shape_points[i - 1][1] and down: | ||
| warnings.warn( | ||
| "Jagged fin shape detected. This may cause small inaccuracies " | ||
| "center of pressure and pitch moment calculations." | ||
| ) | ||
| break | ||
| if shape_points[i][1] < shape_points[i - 1][1]: | ||
| down = True | ||
| i += 1 |
There was a problem hiding this comment.
What is a jagged fin? I looked at OpenRocket documentation, page 27, but I am not sure what they define as a jagged fin. For instance, the images below show three different fins. For Fin 1, we get the jagged fin warning. For Fins 2 and 3, no warning is issued.
Fin 1 - Jagged Fin warning
Fin 2 - No Jagged fin warning
Fin 3 - No Jagged fin warning
According to the raised error, and OR documentation, jagged fins have unstable/unreliable calculations. Are not Fin 2 and Fin 3 also problematic? Should they not be "jagged?"
If that is the case, then is "jagged" a synonym for non-convex?
In any case, I think we should separate this logic into a function in mathutils module depending on what is jagged:
- If jagged is not a synonym of non-convex, then we could create a
check_jagged_regionwhich just implements this, without the warning, and returns a boolean; - If jagged means non-convex, I can implement a
check_convex_regionthat returnsTrueif the region is convex orFalseotherwise.
The class would then just call any of these functions and raise the warning if the condition is False.
There was a problem hiding this comment.
So Open Rocket only checks the "jaggedness" of the first case because I believe that is the only case that makes sense for fins. It would totally be valid to add better checks just like you mentioned
I am not sure though if the assumptions that Barrowman makes for the cp position and clalpha would still hold for case 2 and 3. I personally think they would be too inaccurate.
I don't think this is needed for this PR, but those extra checks you suggested would be great to have in the future. Opening an issue describing those cases would be useful
| prev_idx = int(y1 * 1.0001 / self.span * (points_per_line - 1)) | ||
| curr_idx = int(y2 * 1.0001 / self.span * (points_per_line - 1)) |
There was a problem hiding this comment.
I believe this is something related to numerical issues in Java. This comes directly from Open Rocket.
I removed it because I guess it is not needed for us
Co-authored-by: Gui-FernandesBR <[email protected]>
Co-authored-by: Lucas Prates <[email protected]>
…/RocketPy into enh/free-form-fins
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## enh/generic-surfaces #694 +/- ##
========================================================
+ Coverage 75.81% 76.06% +0.24%
========================================================
Files 98 99 +1
Lines 11092 11249 +157
========================================================
+ Hits 8409 8556 +147
- Misses 2683 2693 +10 ☔ View full report in Codecov by Sentry. |
Pull request type
Checklist
black rocketpy/ tests/) has passed locallypytest tests -m slow --runslow) have passed locallyCHANGELOG.mdhas been updated (if relevant)Description
Free Form Fins have been added!
Every calculation that is related to the free-form fins shape points (essentially almost everything that is calculated inside
evaluate_geometrical_parameters) comes directly from Open Rocket.This means that the free-form fins in RocketPy are modeled exactly like Open Rocket, and only deviate when it comes to some effects related to Mach number variations (especially at supersonic speeds).
Breaking change