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

Move raster footprint functionality into a class #396

Merged
merged 21 commits into from
Feb 16, 2023

Conversation

pjhartzell
Copy link
Collaborator

@pjhartzell pjhartzell commented Feb 3, 2023

Updated Description

Related Issue(s):
None.

Description:
The raster footprint utility often produces incorrect geometry polygons when the data spans the edge of the sinusoidal projection used by MODIS and VIIRS. A fix requires an alternative densification approach and a custom reprojection function. There is currently no convenient way to override these steps in the raster footprint utility since they are: 1) bundled into a single function (densify_reproject_simplify) and 2) that function exists at the end of a chain of function calls.

This PR replicates the functionality of the raster footprint utility in a class (named RasterFootprint) with an eye toward subclassing for selective method overrides. In addition to breaking out individual methods for the densify, reproject, and simplify steps, alternative constructors are provided for footprint creation directly from an href or from a file opened with rasterio (a rasterio DatasetReader object). An alternative densification function is also provided; you can now choose to densify by factor or by distance.

The existing raster footprint functions are retained, but marked as deprecated. Their functionality is fully contained in the new RasterFootprint class and the functions themselves now rely on RasterFootprint.

PR checklist:

  • Code is formatted (run scripts/format).
  • Code lints properly (run scripts/lint).
  • Tests pass (run scripts/test).
  • Documentation has been updated to reflect changes, if applicable.
  • Changes are added to the CHANGELOG.

Original Description (for context)

Related Issue(s):
None.

Description:
The raster footprint utility often produces incorrect and globe-spanning geometry polygons when the data is on the edge of the sinusoidal projection used by MODIS and VIIRS.

  • The nature of the sinusoidal projection precludes data that spans the antimeridian, so the globe-spanning polygons are a symptom of bad vertices.
  • It appears that vertices very close to the projection edge are occasionally projected beyond the antimeridian. Some experimenting with the basic sinusoidal projection equations that are part of this PR indicate a high sensitivity to small changes in the coordinates and/or projection parameters at the projection edges.

This PR provides a fix to create correct raster footprints at the edges of the sinusoidal projection.

Approach:

  • Densely infill the footprint polygon (using the ground pixel size as the densify interval) while still in grid coordinates.
  • Project the densified grid polygon vertices to spherical lon/lat (using equations provided by a MODIS user guide) and throw out any coordinates with an abs(longitude) value greater than 180. This should be able to be done without the custom projection equations, but I am unable to force proj to not wrap the longitude coordinates.
  • By densifying at the ground distance of a single pixel, we should be within at least one pixel of the antimeridian (should the furthermost pixel be thrown out due to reprojection error/instability).

Notes:

  • I converted the existing raster geometry code to a base class, which is then subclassed to recreate the original functionality (the "Default" subclass) and new functionality that handles sinusoidal data (the "Sinusoidal" subclass). This is fine, but it does require some gymnastics to retain the existing function signatures.
  • There is a new, but unused, sinusoidal_pixel_to_grid function in projection.py. It will be used by the MODIS stactools-package and it seemed natural to place it alongside the new sinusoidal_grid_to_lonlat function.
  • I changed the name of an underscored/private function: _densify() to _densify_by_factor(). The name parallels the new _densify_by_distance() function used by the sinusoidal class.

Example:
Current, incorrect behavior (pink) for an h01v11 MODIS tile (MCD15A2H.A2022025.h01v11.061):
image

Proposed, correct behavior (green) via this PR for the same MODIS tile:
image

src/stactools/core/utils/raster_footprint.py Outdated Show resolved Hide resolved
src/stactools/core/utils/raster_footprint.py Outdated Show resolved Hide resolved
src/stactools/core/utils/raster_footprint.py Outdated Show resolved Hide resolved
src/stactools/core/utils/raster_footprint.py Show resolved Hide resolved
src/stactools/core/utils/raster_footprint.py Outdated Show resolved Hide resolved
src/stactools/core/utils/raster_footprint.py Outdated Show resolved Hide resolved
src/stactools/core/utils/raster_footprint.py Outdated Show resolved Hide resolved
src/stactools/core/utils/raster_footprint.py Outdated Show resolved Hide resolved
src/stactools/core/utils/raster_footprint.py Outdated Show resolved Hide resolved
src/stactools/core/projection.py Outdated Show resolved Hide resolved
@gadomski gadomski added the enhancement New feature or request label Feb 9, 2023
@gadomski gadomski added this to the v0.4.4 milestone Feb 9, 2023
Copy link
Member

@gadomski gadomski left a comment

Choose a reason for hiding this comment

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

Do you think we should deprecate the free functions now? Or wait until a later time? Also, looks like rasterio 1.3.5.post1 changed how some subdatasets are named: https://github.com/stac-utils/stactools/actions/runs/4156577655/jobs/7190456348#step:5:32? That feels strange to me...

src/stactools/core/utils/raster_footprint.py Outdated Show resolved Hide resolved
src/stactools/core/utils/raster_footprint.py Show resolved Hide resolved
src/stactools/core/utils/raster_footprint.py Outdated Show resolved Hide resolved
src/stactools/core/utils/raster_footprint.py Outdated Show resolved Hide resolved
src/stactools/core/utils/raster_footprint.py Outdated Show resolved Hide resolved
src/stactools/core/utils/raster_footprint.py Show resolved Hide resolved
@pjhartzell pjhartzell marked this pull request as ready for review February 15, 2023 15:04
@gadomski
Copy link
Member

gadomski commented Feb 15, 2023

Free functions that duplicate class methods are now marked as deprecated since version 0.4.4. Should we bump that to 0.5.0?

We don't have an explicit deprecation policy, but IMO it's best to give at least one release cycle for the deprecations to kick in. So depreacted in 0.4.4, removal planned for 0.6.

Thoughts? Is the different underscoring a breaking change?

I think the problem was due to using rasterio wheels rather than building from scratch (f75b36c). I agree that the test is a bit fragile but #401 should fix it.

src/stactools/core/utils/raster_footprint.py Show resolved Hide resolved
src/stactools/core/utils/raster_footprint.py Outdated Show resolved Hide resolved
src/stactools/core/utils/raster_footprint.py Show resolved Hide resolved
src/stactools/core/utils/raster_footprint.py Outdated Show resolved Hide resolved
src/stactools/core/utils/raster_footprint.py Outdated Show resolved Hide resolved
src/stactools/core/utils/raster_footprint.py Show resolved Hide resolved
tests/core/utils/test_convert.py Outdated Show resolved Hide resolved
- update cli to use new RasterFootprint class method
- test densification methods
@pjhartzell pjhartzell changed the title Make raster footprint geometry for sinusoidal projections work on projection edges Refactor raster footprint into a class Feb 16, 2023
@pjhartzell pjhartzell changed the title Refactor raster footprint into a class Move raster footprint functionality into a class Feb 16, 2023
CHANGELOG.md Outdated Show resolved Hide resolved
@gadomski gadomski enabled auto-merge (squash) February 16, 2023 16:07
@gadomski gadomski merged commit 7416fe6 into main Feb 16, 2023
@gadomski gadomski deleted the update-raster-footprint branch February 16, 2023 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants