BUG: Flight.plotPressureSignals and "StandardAtmosphere" environment pressure calculation #313
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MateusStano
added
Bug
MateusStano
changed the title
8 tasks
Gui-FernandesBR
approved these changes
Dec 31, 2022
Member
Gui-FernandesBR
left a comment
Gui-FernandesBR
left a comment
There was a problem hiding this comment.
Nice one! I verified locally and the code is running as expected.
Regarding line 2833, it was not modified since the first addition of Environment.py file, made 3 years ago by @giovaniceotto . It seems wrong to me, at least one of the definition of the h variable should be deleted. Please notice this problem also happen in line 2815 of the same file.
@giovaniceotto , what do you think? Can we fix that in another PR?
@MateusStano , I believe you can already merge this one 👌
Member
Author
|
Closing this. I opened issue #318 to continue the discussion there. |
Gui-FernandesBR
deleted the
bug/standard_atmosphere_pressure_calculation
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull request type
Description
When working on #290 I found that the
Flight.plotPressureSignalsmethod did not work. This was just because the__calculate_pressure_signalmethod was not being called. Furthermore, when using"StandardAtmosphere"thePressure at Rocket's Altitudeplot raised an error. This error comes from callingEnvironment.pressurewith an array as such:Using a list instead works fine:
I am pretty sure that a Function object should be able to receive an np.array so I think this not working as intended.
One more thing, looking at how the pressure Function is calculated for
StandardAtmosphere. Lines 2832 and 2833 are incoherent. I don't know much about pressure profiles though, so I might be wrong. Maybe @Gui-FernandesBR or @giovaniceotto know more.What was changed
The only thing changed currently in this PR is that the plot for
Pressure at Rocket's Altitudenow converts the np.array into a list when getting the pressure values and also calls__calculate_pressure_signal, which was not doing before so it just did not work. This fixes the plot but there may be more things wrong here.Does this introduce a breaking change?