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

ENH: Improved convective stratiform functions #1314

Merged
merged 74 commits into from
Nov 9, 2022

Conversation

lauratomkins
Copy link
Contributor

I have updated the original convective stratiform functions from Steiner et al. (1995) to include enhancements from Yuter and Houze (1997) and Yuter et al. (2005). The current convective stratiform functions are slow and not easily modified. The new enhancements use the scipy package to make some of the steps significantly faster. The new functions also include many new parameters that can be adjusted for different applications. I have also included an example file that describes the steps of the convective stratiform algorithm, explains the parameters used, and shows several examples.

Any feedback would be greatly appreciated! I've spent a lot of time testing all the different functions so they work as intended. Hoping this can be an easy addition but happy to provide extra information to improve the workflow.

Copy link
Collaborator

@rcjackson rcjackson left a comment

Choose a reason for hiding this comment

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

It looks like you have a couple of files in this commit that don't really need to be there (plot_rhi_sigmet.py). Could you please remove that file? I also suggest moving your hard-coded values to be subroutine keywords.

examples/plotting/plot_rhi_sigmet.py Outdated Show resolved Hide resolved
pyart/retrieve/_echo_class.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@mgrover1 mgrover1 left a comment

Choose a reason for hiding this comment

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

Overall, this looks great! A few suggestions + API suggestions.

pyart/retrieve/echo_class.py Outdated Show resolved Hide resolved
pyart/retrieve/echo_class.py Show resolved Hide resolved
pyart/retrieve/echo_class.py Outdated Show resolved Hide resolved
pyart/graph/convstrat_scheme_plot.py Outdated Show resolved Hide resolved
@mgrover1
Copy link
Collaborator

mgrover1 commented Nov 9, 2022

@lauratomkins - I think with a couple minor changes to the name of the function in the tests/example, the tests should pass.

tests/retrieve/test_echo_class.py Outdated Show resolved Hide resolved
tests/retrieve/test_echo_class.py Outdated Show resolved Hide resolved
@mgrover1
Copy link
Collaborator

mgrover1 commented Nov 9, 2022

@lauratomkins two more spots in the tests - it looks like the last commit fixed it on the docs side! So close!

@mgrover1 mgrover1 changed the title Improved convective stratiform functions ENH: Improved convective stratiform functions Nov 9, 2022
tests/io/test_mdv_grid.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@mgrover1 mgrover1 left a comment

Choose a reason for hiding this comment

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

This all looks great - thanks for making all those changes @lauratomkins ! @rcjackson / @zssherman can you give this another look?

@zssherman
Copy link
Collaborator

@mgrover1 sweet! On my back from lunch then I can take a look!

Copy link
Collaborator

@rcjackson rcjackson left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@mgrover1
Copy link
Collaborator

mgrover1 commented Nov 9, 2022

@zssherman did you still want to take a look? Otherwise, we can go ahead and merge! Great work @lauratomkins !!

@zssherman
Copy link
Collaborator

Looks good to me!

@zssherman zssherman merged commit 996206d into ARM-DOE:main Nov 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants