WIP: Enh/estimate drag coefficients #112
Conversation
Add method to calculate the viscous friction coefficient in Rocket class to be used in the drag coefficient estimations. The calculations required the addition of new parameters to `__init__`.
Added formulas for estimations of the drag coefficients regarding the fins and tail. Also, the method `evaluateForebodyDragCoefficient` still needs to be completed.
Now the drag coefficient estimation is supposedly finished, but testing and validation is still needed.
Some issues with the code were corrected.
Add a new branch in the calculation to correct when Reynolds number is less than 10^4.
The forebody drag coefficient was forgotten on the sum of all drag coefficients. It is now fixed.
rocketpy/Rocket.py
Outdated
| reynolds = ( | ||
| (self.air_density ** 2) |
There was a problem hiding this comment.
@Gui-FernandesBR pointed out that this should be:
| reynolds = ( | |
| (self.air_density ** 2) | |
| reynolds = ( | |
| (self.air_density) |
There was a problem hiding this comment.
We reviewed our references and I believe there is the square, because of an algebraic manipulation. The deduction can be seen below:
Substituting the values yields:
What do you think, @Gui-FernandesBR @giovaniceotto?
There was a problem hiding this comment.
Reynolds is a dimensionless quantity... Just change "kinematic" for "dyanamic" in the first image you've sent and it's going to work then, it's a honest and simple mistake.
There was a problem hiding this comment.
Yes, I still believe there is a slight error here. I hope this source, which has both formulas (using dynamic and kinematic viscosity) can help settle the debate: https://www.engineeringtoolbox.com/reynolds-number-d_237.html
There was a problem hiding this comment.
Ok, I understand now what is going on now, apparently our main reference had a typo on the definition of the Reynolds number and we just trusted it. Thank you both for pointing it out and helping us understand the problem. It will be fixed soon.
|
Hey @Lucas-KB and @lucasfourier , great to have such an interesting contributions from you guys. I made some commetns in order to help your review before pulling the code to the develop branch, can't wait to see that happening! |
Documentation was fixed and some lines of code were added to make sure plots worked fine.
| @@ -174,7 +188,7 @@ def tests_export_eng_asserts_exported_values_correct(solid_motor): | |||
| "0", | |||
| "{:2.3}".format(grain_mass), | |||
| "{:2.3}".format(grain_mass), | |||
| "RocketPy" | |||
| "RocketPy", | |||
There was a problem hiding this comment.
Should that comma exist? I believe it was created automatically, but I'm concerned if this going to create a bug or not. @Lucas-KB @lucasfourier
|
|
||
| # In the formula, instead of span, it should be the distance between the | ||
| # midpoint of the root and the tip of the fin, but we are using | ||
| # span for now. |
There was a problem hiding this comment.
If you consider a right-trapezoid, it should be sqrt(span² + (Rc - tc)²/4). If you want to be more abstract, could get the angle bettwen span and rocket axis and use it on the law of cosines.
In my opinion, you don't need to make this correction on the code, I'm approving the pull request with the current approach. My point here is that it's not so hard to determine the distance you said, I think it's just about geometry. Reccomend save this idea for later.
|
Quick question. Is this implementation compatible with rockets which have multiple "tails" (which may actually be diameter variation) and multiple fin sets? |
|
As I just made with #110, I suggest merging this PR and then start another to (1) create the examples and (2) better documation, if the plan is still to do do 1 and 2 before merging. @giovaniceotto sorry, I took a look but I can't answer your last question precisally. @Lucas-KB may know it. |
Gui-FernandesBR
added
the
Enhancement
Gui-FernandesBR
added
NewFeature
and removed
Enhancement
Gui-FernandesBR
changed the title
|
TODO (@Lucas-KB & @lucasfourier):
|
Gui-FernandesBR
requested review from
MateusStano
and removed request for
giovaniceotto
|
I'm quoting @MateusStano here : (automatic translated through google translate)
|
|
Getting this PR back online will be no easy task. That being said, I do not believe closing it as a viable option. We need someone to take over the great work left by @Lucas-KB and @lucasfourier. |
Yeah agreed on that. I think it would be good if we could convert it to a draft PR since the code is not finished yet. But I'm afraid only @Lucas-KB can perform such conversion. Also, I think we need not only new people to work on aerodynamic developments but also need to improve our development and integration process. Similar to Environment Analysis, we should list all implementations before start cododing, and try our best to separate everything into small pull requests so it can be better managed. |
|
Thinking again, there's no problem if we close the PR since github will still remain information regarding everything. There's a difference between closing and deleting. The only thing we must me carefull is that we should not delete head branch! Therefore I believe we should go ahead and close the PR in the next few days |
Pull request type
Please check the type of change your PR introduces:
Pull request checklist
Please check if your PR fulfills the following requirements, depending on the type of PR:
ReadMe, Docs and GitHub maintenance:
Code base maintenance (refactoring, formatting, renaming):
black rocketpy) has passed locally and any fixes were madepytest --runslow) have passed locallyCode base additions (for bug fixes / features):
black rocketpy) has passed locally and any fixes were madepytest --runslow) have passed locallyWhat is the current behavior?
The user has to provide a drag curve as a function of the Mach number.
What is the new behavior?
The user can supply additional rocket parameters instead of the curves themselves, but the old behavior can still be achieved.
Does this introduce a breaking change?
Other information