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

Limit shading of CIN #1139

Merged
merged 4 commits into from Jan 10, 2020
Merged

Limit shading of CIN #1139

merged 4 commits into from Jan 10, 2020

Conversation

zbruick
Copy link
Contributor

@zbruick zbruick commented Aug 27, 2019

Description Of Changes

Adds default limits to shading for CIN to not shade below the LCL or above the EL. As a result of the default being True, dewpoint is now a required input variable, in order to calculate LCL and EL.

Also, a more realistic (and somewhat complicated) test profile was added to allow for easier identification of edge cases in future development. The previous one didn't include dewpoint, which is now needed.

Checklist

@zbruick zbruick added Area: Plots Pertains to producing plots Type: Enhancement Enhancement to existing functionality labels Aug 27, 2019
@zbruick zbruick force-pushed the cin_shade branch 3 times, most recently from 788fdc7 to eac9bfa Compare August 27, 2019 19:32
@dopplershift
Copy link
Member

Changing to make dewpoint required is an API change, so we're going to need to ponder this a bit further on if there are ways to make it non-breaking.

@zbruick
Copy link
Contributor Author

zbruick commented Aug 29, 2019

Yeah I though that might be an issue. I only added the dewpoint requirement to get the LCL trimming. The EL could be found with finding the final intersection between the temperature and profile paths. But CIN is not calculated below the LCL, so it doesn't make sense to shade below that.

metpy/plots/skewt.py Outdated Show resolved Hide resolved
@zbruick zbruick force-pushed the cin_shade branch 2 times, most recently from 419e5ed to fd0887f Compare September 3, 2019 15:10
@zbruick zbruick force-pushed the cin_shade branch 3 times, most recently from d13b007 to 4fd050c Compare September 26, 2019 20:32
@dopplershift dopplershift added this to the 0.12 milestone Oct 2, 2019
@zbruick zbruick force-pushed the cin_shade branch 4 times, most recently from dba88ac to ccc917b Compare October 8, 2019 17:14
@zbruick zbruick force-pushed the cin_shade branch 2 times, most recently from 8f8b5f4 to 90ecf50 Compare October 30, 2019 16:37
@kgoebber
Copy link
Collaborator

kgoebber commented Nov 2, 2019

Could we use potential temperature to help us find the LCL? We would have to assume that initial ascent is dry adiabatic so computed potential temperature from the prof variable should yield the same value until saturated rising occurs. It wouldn't perfectly get to the LCL as they could occur between layers, but could be a way not to break the API.

@kgoebber
Copy link
Collaborator

kgoebber commented Nov 4, 2019

As I have been thinking more about this, I'm wondering if we need to change the output of parcel_profile to add in the LCL level (T and p) explicitly. Without that in there, the parcel path is not really correct as plotted since we don't plot the dry adiabatic ascent to that level. This would be a big API break, but if we're going to do it, best to do it before 1.0

Then we would be able to use Potential Temperature in helping to shade CIN as desired, since the LCL would be explicit inherent in the data itself.

@zbruick
Copy link
Contributor Author

zbruick commented Nov 4, 2019

I agree that the current parcel_profile is deceptive, as we've run into issues where the lack of the LCL inclusion was causing issues with CAPE/CIN, and I then updated all CAPE/CIN functions to use parcel_profile_with_lcl.

Would your suggestion mean that we deprecate the name parcel_profile_with_lcl and move that function into the parcel_profile namespace? We wouldn't need both functions any longer then if I understand correctly?

@kgoebber
Copy link
Collaborator

kgoebber commented Nov 4, 2019

Yes, I think so. And somehow I missed that function going in the 0.9 release.

@zbruick
Copy link
Contributor Author

zbruick commented Nov 4, 2019

Pitching this over to @dopplershift - is there any remaining purpose for the parcel_profile function as is? This would obviously be a complicated API break (although some of the infrastructure is already in place with the future module), with a short deprecation cycle for 1.0. But I'm +1 for reducing confusion and increasing scientific accuracy.

In the meantime, not sure what happens with this PR. Can be closed and @kgoebber if you want to do the potential temperature version of this at some point, that could work. Not sure if my timeline is going to work to keep this updated with the proposed changes.

@dopplershift dopplershift added the Type: API Change Changes to how existing functionality works label Dec 24, 2019
@dopplershift
Copy link
Member

So regarding parcel_profile vs. parcel_profile_with_lcl: the former is much simpler to use because it only returns the parcel temperatures at the pressures given. IMO, it's much easier to use for plotting and things like that. At the very least, I'd want to hear a good argument for ripping it out (which would likely be a 2.0 item since we're on the doorstep of 1.0)

On the limits, what I don't like is that we have add a required argument that is only used if we pass True to another argument. I think what I'm going to do is make Td a trailing last optional argument. If it's given, it does the limiting. If no Td is given, then no limiting. It also keeps the leading part of the method signature the same as shade_cape (p, t, t_parcel) which I think is better for the overall library API design than only shade_cin having (p, t, td, t_parcel).

@dopplershift
Copy link
Member

And maybe most importantly, by putting dewpoint as a trailing argument, this is no longer an API break.

@dopplershift dopplershift removed the Type: API Change Changes to how existing functionality works label Dec 24, 2019
Make dewpoint the optional last argument, and use its presence to enable
the limiting behavior. This preserves the previous API, keeps things
symmetric with shade_cape(), and keeps us from requiring passing
dewpoint when we don't need or use it.
@dopplershift
Copy link
Member

And since this is no longer an API break that I want to start in 0.12, I'm going to push to 1.0 to allow some time for comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Plots Pertains to producing plots Type: Enhancement Enhancement to existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add limiting to shade CAPE/CIN
3 participants