RocketPy

RocketPy copied to clipboard

Pull request type

Please check the type of change your PR introduces:

• [x] Code base additions (bugfix, features)

Pull request checklist

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

• ReadMe, Docs and GitHub maintenance:

  • [x] Spelling has been verified
  • [x] Code docs are working correctly

• Code base maintenance (refactoring, formatting, renaming):

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

• Code base additions (for bug fixes / features):

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

What is the new behavior?

Please check the newst plot regarding wind heading

Does this introduce a breaking change?

• [ ] Yes
• [X] No

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

@Gui-FernandesBR, please provide an image of the resulting plot.

I will be honest with you. Since this was the only plot left to implement, I tried it yesterday but then gave up due to inherent problems related to ploting wind direction (the transition from 360 back to 0 degrees)

@Gui-FernandesBR, please provide an image of the resulting plot.

I will be honest with you. Since this was the only plot left to implement, I tried it yesterday but then gave up due to inherent problems related to ploting wind direction (the transition from 360 back to 0 degrees)

I also was concerned with this wrap situation, but at the end idk why i just followed through and made the calculations anyway.

I will show the result, do you believe it makes sense?

I just tried to plot the profile on a polar plot but it seems it didn't work well

Doing some research now (for the nth time), I found an interesting solution. I suggest looking at the second answer from this stack overflow post: https://stackoverflow.com/questions/16058096/conditionally-draw-connecting-lines-in-matplotlib

I also tried the polar plot... It looked a little better, but I was not pleased with the result. You need to give the value in radians to get an accurate plot (even though the axis label is in degrees)

Doing some research now (for the nth time), I found an interesting solution. I suggest looking at the second answer from this stack overflow post: https://stackoverflow.com/questions/16058096/conditionally-draw-connecting-lines-in-matplotlib

I also tried the polar plot... It looked a little better, but I was not pleased with the result. You need to give the value in radians to get an accurate plot (even though the axis label is in degrees)

I'm sorry, I can't see how does that work for our case

Refreshed results:

In the example you gave, I do not see lines crossing from 360 to 0 deg. What happens to the curves when this happens?

@giovaniceotto the same old behavior would happen as I could not understand how to apply the wrapper provided by the stack overflow commentaries. There's some wind profiles in the picture that presents this behavior, see below where I try to highlight one:

But please take in mind that the red lines are not being evaluated based on the wind heading profiles. Instead, I'm calculating the mean of windVelocityX and windVelocityY profiles, and with them I calculate the wind heading profile of mean wind velocity components. In other words, the red plot is already the correct and can me used, but all the other lines not.

Maybe we could implement only what is right in this PR and left the other part to be solved apart.

This 0º-360ºwrapper problem is something that is currently already present in the Environment class as well, which has been used for years without any issue being raised.

Current result (still don't solve wrap problem, but it will already work on most of cases)

Just in order to prioritize other tasks, I'll move forward with this PR using the current approach, and in the features perhaps we can solve the other problems that were raised (I am raising an issue for that).

The simple image in this comments illustrates that current a simpler way to evaluate the mean wind heading profile were used. The only part that is a concern is regarding plotting, one that might be problematic when the wind heading varies more than 180º from one to next pressure level.