contrailcirrus / pycontrails

Python library for modeling contrails and other aviation climate impacts
https://py.contrails.org/
Apache License 2.0
57 stars 16 forks source link

Feature/upper bound polygons #192

Closed thabbott closed 5 months ago

thabbott commented 5 months ago

Closes #191

Changes

Adds a lower_bound keyword argument to polygon.find_multipolygon that controls whether threshold is used as a lower or upper bound on the value of regions inside polygons. The default value (lower_bound=True) preserves existing behavior (using threshold as a lower bound).

To find cooling polygons where threshold is an upper bound on EF, pass lower_bound=False:

threshold = -1e7
mp = polygon.find_multipolygon(ef, threshold, lower_bound=False, ...)

Features

Add ability to use polygon.find_multipolygon to find regions with values below a threshold.

Tests

Reviewer

@nickmasson, @mlshapiro

thabbott commented 5 months ago

To make this drop-in usable, however, we should bubble up the lower_bound param to other methods that use this method in composing their behavior.

Oops, you're obviously right.... I'll go through the rest of the code base tomorrow and make changes as needed.

How is a zero value within the threshold range expected to behave? If a contrail is not formed, will it be a nan value in the grid, or will it be zeroed?

ef and contrail_age will be zero if a contrail isn't formed. Many other variables will be nan, though; there are some details in the CoCiP docstring. So, polygons created with a negative threshold and lower_bound=True will include regions where no contrails form. I can clarify this in relevant places in docstrings as I address your other point.

thabbott commented 5 months ago

The most recent changes surface the lower_bound keyword in MetDataArray.to_polygon_feature and MetDataArray.to_polygon_feature_collection. For completeness, I made a similar change in MetDataArray.to_polyhedra. @nickmasson, do these changes give you the tooling you need to generate polygons with negative EF?