Skip to content
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

Bug fix/wake vortex #80

Merged
merged 15 commits into from
Aug 16, 2023
Merged

Bug fix/wake vortex #80

merged 15 commits into from
Aug 16, 2023

Conversation

roger-teoh
Copy link
Collaborator

Closes #XX

Changes

Address unit inconsistency in turbulent_kinetic_energy_dissipation_rate function as identified by Wessel99, and update wake_vortex.py

Breaking changes

Features

Fixes

  • Bug fix in turbulent_kinetic_energy_dissipation_rate in wake_vortex.py
  • Improve docstrings in wake_vortex.py
  • Rename functions in wake_vortex.py by removing "get" at the start of each function.

Internals

Tests

  • QC test passes locally (make test)
  • CI tests pass

Reviewer

Select @ reviewer

@roger-teoh roger-teoh requested a review from zebengberg August 15, 2023 11:00
@roger-teoh roger-teoh self-assigned this Aug 15, 2023
@roger-teoh
Copy link
Collaborator Author

@zebengberg You might need to update the .json files in the unit testing of CoCiP outputs.

@mlshapiro mlshapiro requested review from mlshapiro and removed request for zebengberg August 15, 2023 13:19
@mlshapiro mlshapiro self-assigned this Aug 15, 2023
Copy link
Contributor

@mlshapiro mlshapiro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Roger! I've reviewed, and it looks good. Thanks for the documentation improvements.

I'll add the new reference to the bibliography on Zotero and make the citation.

@zebengberg I'll merge this into main once I'm done.

pycontrails/models/cocip/wake_vortex.py Outdated Show resolved Hide resolved
@zebengberg
Copy link
Contributor

Great work @roger-teoh!

@mlshapiro: can you also update the changelog? If the static cocip test files need to be significantly updated, I think you should cut a new release (v0.45.1).

@mlshapiro mlshapiro merged commit 09600a7 into main Aug 16, 2023
@mlshapiro mlshapiro deleted the bug-fix/wake-vortex branch August 16, 2023 04:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants