Gtc 3025/filter alerts by forest #170

solomon-negusse · 2024-10-30T20:19:20Z

Pull request checklist

Please check if your PR fulfills the following requirements:

Make sure you are requesting to pull a topic/feature/bugfix branch (right side). Don't request your master!
Make sure you are making a pull request against the develop branch (left side). Also you should start your branch off our develop.
Check the commit's or even all commits' message styles matches our requested structure.
Check your code additions will fail neither code linting checks nor unit test.

Pull request type

Please check the type of change your PR introduces:

What is the new behavior?

Adds ability to filter DIST alerts layer by forest parameters (tree cover loss, density and height)

Does this introduce a breaking change?

Yes
No

Other information

codecov-commenter · 2024-10-31T17:40:51Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 58.82353% with 21 lines in your changes missing coverage. Please review.

Project coverage is 79.85%. Comparing base (bc349c0) to head (693e0c2).

Files with missing lines	Patch %	Lines
app/routes/titiler/umd_glad_dist_alerts.py	15.00%	17 Missing ⚠️
app/settings/globals.py	66.66%	4 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##              dev     #170      +/-   ##
==========================================
- Coverage   80.29%   79.85%   -0.45%     
==========================================
  Files          61       61              
  Lines        2005     2050      +45     
==========================================
+ Hits         1610     1637      +27     
- Misses        395      413      +18

Flag	Coverage Δ
unittests	`79.85% <58.82%> (-0.45%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

danscales

Looks good, but I'm just wondering about my questions related to the TCL mask.

danscales · 2024-11-04T18:13:08Z

app/routes/titiler/algorithms/dist_alerts.py

+            # 2018, but has tree cover height (2020) that meets the forest threshold, the pixel meets
+            # the forest criteria for alerts.
+            mask *= (
+                self.tree_cover_loss_data.array[0, :, :] >= self.tree_cover_loss_mask


Shouldn't the mask be logically negated in this case? I think the mask is True is we want to include the alerts. But we don't want to include an alert if this pixel has a loss year of 2021, for example.

But separately, in Sarah's document, they said that we shouldn't remove alerts for pixels that have loss year of 2022 or 2023, because we actually want those alerts to show up in our history. I.e. the loss may have been the cause of the alert we want to show.

So, it somewhat seems like you only want to ever remove alerts here that have a loss year of exactly 2021. If loss year is <= 2020, then that is covered by the tree cover height, as you said, so you ignore that too.

Yes, thanks for catching - this needs to be self.tree_cover_loss_data.array[0, :, :] > self.tree_cover_loss_mask.
I included this filter to allow them the flexibility of masking out the later tree cover loss years (2022, 2023) as needed.

~~And by default, it's going to filter out just 2021.~~ actually didn't implement this

OK, maybe you can give a comment on the meaning of tree_cover_loss_mask above? So, it's actually the highest loss year that should be used to exclude alerts. And by default we never use TCL from 2020 or less (including 0) to exclude alerts. So, if tree_cover_loss_mask is 2021, then we only use loss year 2021 to exclude alerts.

danscales · 2024-11-04T18:14:35Z

app/routes/titiler/umd_glad_dist_alerts.py

+
+    if tree_cover_loss_cutoff:
+        with COGReader(
+            f"s3://{DATA_LAKE_BUCKET}/umd_tree_cover_loss/v1.10.1/raster/epsg-4326/cog/default.tif"


Put in a TODO here to fix the version name (to v1.11) once we go from staging to production?

How often will we need to change the code? Should this be a config variable somewhere?

TCL is updated once a year I believe, so not that frequent but still good idea to not hardcode it here.

@gtempus: I hit what's potentially a terraform bug setting an env variable that's a json string with the dataset versions - same variable that works fine with the docker-compose file. So, for now I have moved out the dataset versions into the global config file where we'll have to update it (693e0c2) as interim solution and opened an issue in terraform aws provider that I hope will get sorted out.

This is a good interim solution, @solomon-negusse. Thanks!
It's much better than having to dig into the codebase. 😄
Bummer about the terraform bug. 😦

gtempus

I appreciate all the attention to documentation (descriptions and tests), @solomon-negusse! 🚀
I just was wondering about the hardcoded versions in the URL paths of the layers.

gtempus · 2024-11-05T15:48:44Z

app/routes/titiler/algorithms/dist_alerts.py

+        if self.tree_cover_loss_mask:
+            # Tree cover loss before 2020 can't be used to filter out pixels as not forest
+            # if they had tree cover loss. Instead we use tree cover height taken that year
+            # is used as source of truth.


Thank you for the informative comment, Solomon. 💪

gtempus · 2024-11-05T15:55:59Z

app/routes/titiler/umd_glad_dist_alerts.py

+    tree_cover_density: Optional[int] = Query(
+        None,
+        ge=0,
+        le=100,
+        description="Alerts in pixels with tree cover density (in percent) below this threshold won't be displayed. `umd_tree_cover_density_2010` is used for this masking.",
+    ),
+    tree_cover_height: Optional[int] = Query(
+        None,
+        description="Alerts in pixels with tree cover height (in meters) below this threshold won't be displayed. `umd_tree_cover_height_2020` dataset in the API is used for this masking.",
+    ),
+    tree_cover_loss_cutoff: bool = Query(
+        False,
+        ge=2021,
+        description="""This filter is to be used in conjunction with `tree_cover_density` and `tree_cover_height` filters to detect only alerts in forests, by masking out pixels that have had tree cover loss prior to the alert.""",
+    ),


Nice descriptions here, @solomon-negusse ! 💪

gtempus · 2024-11-05T15:56:54Z

app/routes/titiler/umd_glad_dist_alerts.py

+
+    if tree_cover_loss_cutoff:
+        with COGReader(
+            f"s3://{DATA_LAKE_BUCKET}/umd_tree_cover_loss/v1.10.1/raster/epsg-4326/cog/default.tif"


How often will we need to change the code? Should this be a config variable somewhere?

gtempus · 2024-11-05T15:57:35Z

tests/titiler/test_alerts_algo.py

@@ -104,3 +149,33 @@ def test_encoded_rgba():

    # test high confidence in alpha channel
    assert rgba.array[3, 154, 71] == 8
+
+
+def test_forest_mask():


Thank you for continuing to improve the test coverage, @solomon-negusse ! ❤️

danscales

Looks good!

danscales · 2024-11-05T17:26:55Z

app/routes/titiler/algorithms/dist_alerts.py

+    tree_cover_height_data: Optional[ImageData] = None
+
+    # the highest loss year that is used to exclude alerts for
+    # the purpose of showing only alerts in forests


Maybe in a following change you can just add the extra sentence "Should be 2021 or later, since 2020 tree height data will cover tree loss before 2021.".

Will do. I have already added a validator to enforce that in the endpoint itself here too: https://github.com/wri/gfw-tile-cache/blob/gtc-3025/filter-alerts-by-forest/app/routes/titiler/umd_glad_dist_alerts.py#L65

add forest filter layers

c66efd2

solomon-negusse marked this pull request as draft October 30, 2024 20:19

solomon-negusse added 2 commits October 31, 2024 20:14

add DIST alerts forest mask test

57d86bb

add tcl mask that was left behind

a8ffd95

solomon-negusse added 3 commits November 1, 2024 12:52

Merge branch 'dev' into gtc-3025/filter-alerts-by-forest

78af193

clean up tree cover loss logic and comment

32857c6

simplify tree cover loss doc

7f968e8

solomon-negusse marked this pull request as ready for review November 1, 2024 16:42

solomon-negusse requested review from danscales, gtempus, dmannarino and manukala6 November 1, 2024 16:42

danscales reviewed Nov 4, 2024

View reviewed changes

solomon-negusse added 2 commits November 4, 2024 22:13

fix TCL mask for DIST alert

6fe4291

update docs for clarity; add todo comment for version update in prod

454d840

solomon-negusse requested a review from danscales November 5, 2024 09:33

gtempus approved these changes Nov 5, 2024

View reviewed changes

danscales approved these changes Nov 5, 2024

View reviewed changes

get DIST alerts filter dataset/versions

693e0c2

solomon-negusse merged commit 2e04a59 into dev Nov 7, 2024
5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Gtc 3025/filter alerts by forest #170

Gtc 3025/filter alerts by forest #170

solomon-negusse commented Oct 30, 2024 •

edited

Loading

codecov-commenter commented Oct 31, 2024 •

edited

Loading

danscales left a comment

danscales Nov 4, 2024

solomon-negusse Nov 4, 2024

solomon-negusse Nov 4, 2024 •

edited

Loading

danscales Nov 4, 2024

danscales Nov 4, 2024

gtempus Nov 5, 2024

solomon-negusse Nov 5, 2024

solomon-negusse Nov 7, 2024 •

edited

Loading

gtempus Nov 7, 2024

gtempus left a comment

gtempus Nov 5, 2024

gtempus Nov 5, 2024

gtempus Nov 5, 2024

gtempus Nov 5, 2024

danscales left a comment

danscales Nov 5, 2024

solomon-negusse Nov 5, 2024

Gtc 3025/filter alerts by forest #170

Gtc 3025/filter alerts by forest #170

Conversation

solomon-negusse commented Oct 30, 2024 • edited Loading

Pull request checklist

Pull request type

What is the new behavior?

Does this introduce a breaking change?

Other information

codecov-commenter commented Oct 31, 2024 • edited Loading

Codecov Report

danscales left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

solomon-negusse Nov 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

solomon-negusse Nov 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gtempus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danscales left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

solomon-negusse commented Oct 30, 2024 •

edited

Loading

codecov-commenter commented Oct 31, 2024 •

edited

Loading

solomon-negusse Nov 4, 2024 •

edited

Loading

solomon-negusse Nov 7, 2024 •

edited

Loading