-
Notifications
You must be signed in to change notification settings - Fork 26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
possible bug in int2P2H #43
base: main
Are you sure you want to change the base?
Conversation
the val calculated by the end flag did not make sense as it was creating a pressure value of ~102 at a higher altitude, which was greater than the previous value of 100 at a lower altitude
Reviewer's Guide by SourceryThis PR fixes a bug in the interpolation calculation within the intP2H function. The change modifies the formula used to calculate pressure values at higher altitudes by changing a subtraction operation to an addition, which resolves an issue where pressure values were incorrectly increasing with altitude. Class diagram for intP2H function changesclassDiagram
class intP2H {
- lvls
- hgt
- gph
- tmp
- vpr
- cdic
- verbose
+ intP2H(lvls, hgt, gph, tmp, vpr, cdic, verbose)
+ // Modified calculation for pressure values
}
note for intP2H "The calculation for pressure values at higher altitudes was changed from subtraction to addition."
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
PR summaryThis Pull Request addresses a potential bug in the SuggestionTo ensure the robustness of this fix, it would be beneficial to add or update unit tests in Disclaimer: This comment was entirely generated using AI. Be aware that the information provided may be incorrect. Current plan usage: 0.00% Have feedback or need help? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @Merdele - I've reviewed your changes - here's some feedback:
Overall Comments:
- Please provide a more detailed explanation of the interpolation formula and why changing the sign fixes the pressure-altitude relationship.
- Consider adding test cases that demonstrate the original bug and verify the fix maintains correct pressure-altitude relationships across different scenarios.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
the val calculated by the end flag did not make sense as it was calculating a pressure value of ~102 at a higher altitude, which was greater than the previous value of 100 at a lower altitude in test_calc.py
Summary by Sourcery
Bug Fixes: