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

Add parallax correction via new ParallaxCorrectionModifier #1904

Merged
merged 129 commits into from Jul 28, 2022

Conversation

gerritholl
Copy link
Collaborator

@gerritholl gerritholl commented Nov 29, 2021

Add a parallax correction to Satpy.

This is a team effort based on work by Simon Proud (@simonrp84), Xin Zhang (@zxdawn), Ulrich Hamann (@uhamann), Jussi Leinonen (@jleinonen), myself, and others.

We have made good progress during the Pytroll Contributor Week 29 November - 3 December 2021. More work was done in December 2021 and January 2022.

  • Closes Add parallax correction #1845
  • Tests added
  • Fully documented
  • Add your name to AUTHORS.md if not there already
  • Add more tests where
    • source and target resolutions are very different
    • areas are only partly overlapping
    • areas are not overlapping at all
    • areas are the same
  • get reliable reference values for unit tests where this value is not trivial
  • Investigate if other resamplers resolve the problems at poles and antimeridian
  • Make resampler and search radius configurable
  • Add easy to use modifier interface
  • Get orbital parameters from somewhere else than the metadata of the CTH dataset. This information is not included with nwcsaf-geo and arguably should come from the dataset to be corrected, not from the CTH (which theoretically doesn't even need to come from the same satellite). See Add parallax correction via new ParallaxCorrectionModifier #1904 (comment).
  • Make sure heights are in metre
  • Add test to cover the case where clouds are moved out of the area of interest completely.
  • Improve speed (although there are no formal performance tests yet, small tests show the correction is very slow).
  • Fix bug when loading corrected and uncorrected datasets simultaneously.
  • Check why brightness is different for parallax corrected image and fix this.
  • Make internal resampling of CTH more flexible (rather than hardcoding method and parameters)
  • Make resampling in modifier more flexible, actually using the provided parameters.

Deferred:

  • Not critical: implement checks for warning or error if areas do not or only partially overlap. This relies on intersection calculation never completes, infinite loop with 100% CPU pyresample#329 to be fixed first. Without those checks, applying parallax correction may yield unexpected results.
  • Add diagnostic tool to get the displacement (distance & direction)
  • Expose and document interface to intermediate swathdefinition as public API
  • Add test for when parallax correction shifts a cloud across the antimeridian or pole (or document that this is not supported)
  • Allow explicitly passing satellite location for datasets where this information is not included (either when no information provided at all, or when estimate based on TLE and skyfield is undesirable)
  • Allow obtaining satellite location or viewing angles from the metadata not only of CTH but also of other datasets. This would require those to be passed to the modifier as optional prerequisites.
  • Add performance tests (in benchmark directory?) with real scenes (could be a different PR)
    • Check dask-awareness
    • Check/control for what area parallax is calculated when scene is resampled, but modifier interface is used
    • Confirm that dask reuses calculations, such as when multiple channels are corrected
    • Check if further optimisations are needed for multiple channels, such as avoiding recalculating the dask graphs

Jussi Leinonen and others added 9 commits May 5, 2021 11:12
Started working on unit tests for the parallax correction.  Not much
content yet.  Everything is likely to change completely.

Added a dummy implementation, failing, just so that the unit test gets
beyond the initial import.
…rection

Merge history of monti pytroll parallax branch into satpy to preserve
history of parallax.py
Merge first version of parallax.py into satpy, adding (some) docstrings
and fixing pep8 issues.  From here we can try to make a composite out of
it and match it up with the unit test already added.
@gerritholl gerritholl added this to In progress in PCW Autumn 2021 Nov 29, 2021
Adapt a dependency from the parallax module adopted from meteoswiss.  I
expect adding this module is temporary.  I don't really know what it's
for.
To AUTHORS.md, add Jussi Leinonen due to his parallax contributions.
Also add affiliation for DWD authors (request from DWD).
Add unit tests for the low-level parallax_correct function, according to
an interface I believe it should have, but that it doesn't have.  Most
of the unit tests are failing.

The higher level class is not unit tested yet because I don't understand
what it's supposed to do exactly.  Probably much of it needs to be
deleted.
Add more unit tests for the parallax correction.  Started with unit
tests for the class.  They're failing, but I don't know why.
Remove some unused code.  Will remove more later.
@gerritholl
Copy link
Collaborator Author

Test to calculate parallax correction for New England, 2021-12-29 20:01:17Z.

Uncorrected RGB:

20211129200117-GOES-16-abi-new-england-1000-natural_color-nocorr

Corrected RGB:

20211129200117-GOES-16-abi-new-england-1000-natural_color-corr

Uncorrected C13:

20211129200117-GOES-16-abi-new-england-1000-C13-corr

Corrected C13:

20211129200117-GOES-16-abi-new-england-1000-C13-nocorr

Difference C13:

parallax-delta-C13

Code for test:

@gerritholl
Copy link
Collaborator Author

gerritholl commented Dec 1, 2021

My understanding so far.

Consider:

parallax

  • For location A", the satellite reports a cloud that is actually at location A. The location A" is actually clear-sky, but we have no way of knowing that.
  • For location A, the satellite reports a cloud that is actually at location A'.
  • For location A', the satellite observes clear-sky, but location A' is actually cloudy.

The forward parallax correction can calculate that for position A", a cloud seen at height h is in reality position A, and that at location A, a cloud at height h is in reality at location A'. We can create a SwathDefinition that assigns a new lat/lon for each and use that. Or we can — or must? — do another step and reassign the pixel values. Meaning:

  • For location A, we know that this is the actual location of the cloud reported at location A" — so at this location, we assign the pixel value that was reported for location A".
  • Likewise, for location A', we know that A' corresponds to the true location of the pixel reported at location A, so here we assign the pixel value that was reported for location A.

This reassignment of pixels, if I understand correctly, is done by this PR by calculating an inverse transformation using a convolution (I don't understand the details yet here), resampling to this, and then reassigning the original base area:

corrector = ParallaxCorrection(base_area)
plax_corr_area = corrector(cloud_top_height)  # SwathDefinition of same size as base_area
local_scene = global_scene.resample(plax_corr_area)
local_scene[some_dataset].attrs["area"] = base_area

This somewhat abuses the concept of resampling and I don't understand it fully yet, but it appears to work for a high resolution area, as the images in the previous comment show...

Thoughts welcome!

Add another parallax correction unit test that uses a much tighter area.
@gerritholl
Copy link
Collaborator Author

I selected a clear sky area in the range 71.9-73.2°W, 43.6–45.0°N, and plotted the difference in channel 13 before and after applying parallax correction in the way I understand it to be correct (see previous comment). This exhibits unphysical artefacts with differences of up to 4–5 K:

delta-parallax-clearsky

@jleinonen
Copy link

I pushed an alternative implementation as parallax2.py in order to avoid merge conflicts. If we decide to use it then we should eventually replace parallax.py with it.

Changes:

  • Eliminate the dependence on projection.py (this too can be deleted if we adopt this version)
  • Replace the RBF interpolation with NumpyBilinearResampler from Pyresample
  • Do the interpolation on the correction to (lon,lat) instead of (lon,lat) itself

Things remaining to do/consider:

  • The radius_of _influence parameter to the resampler is hardcoded - can/should we detect this automatically?
  • Testing different scenarios: source resolution much higher/lower than the target resolution, source and target areas not fully overlapping
  • Testing on real scenes and evaluating accuracy and performance
  • Should we replace NumpyBilinearResampler with XArrayBilinearResampler?
  • What about nearest neighbor interpolation?

Merge the simplification and improvement from the previous commit by
Jussi back into parallax.py, and delete the unused and unneedded
projection.py as well as an unused convolve import.
Parameterise some of the fixtures used in the parallax unit tests.
Still needs some more testing but we're making progress.
@gerritholl
Copy link
Collaborator Author

With the latest update, the Δ looks much better:

delta-parallax-clearsky

Note the change in the scale. The high values near the bottom may be due to a cloud that is just outside the field of view.

@gerritholl
Copy link
Collaborator Author

We also need to test what happens at the antimeridian. And at the poles? Although most critical for geostationary applications in (northern) Europe, people may want to use it for polar orbiters too.

Add more unit tests for the parallax correction, or rather more
parameters to use in the unit tests.  The parallax correction fails at
the antimeridian.
In the parallax correction documentation, describe the version when it
was added in the text.
Put the note on units in the docstring rather than in a comment.
In docstrings, write out the word "degrees" rather than using the symbol
"°".
Add some more flexibility in the ParallaxCorrection class.
Allow user to set the radii of influence in the two nearest neighbour
resampling steps in the parallax correction.
Copy link
Member

@mraspaud mraspaud left a comment

Choose a reason for hiding this comment

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

I just have three minor comments (on top of the one left from before), then I think we can merge. Great job!

doc/source/modifiers.rst Outdated Show resolved Hide resolved
satpy/utils.py Outdated Show resolved Hide resolved
doc/source/modifiers.rst Show resolved Hide resolved
Remove a duplicate line from the modifiers documentation.  Change the
maybe_attempt_tle parameter in ``utils.get_satpos`` to ``use_tle``.
In test_utils, adapt unit test for the changed parameter name in
get_satpos.
Copy link
Member

@mraspaud mraspaud left a comment

Choose a reason for hiding this comment

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

LGTM

satpy/modifiers/parallax.py Outdated Show resolved Hide resolved
Simplify the documentation for the forward_parallax function based on a
suggestion by @ghiggi.
Copy link
Contributor

@ghiggi ghiggi left a comment

Choose a reason for hiding this comment

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

Hey @gerritholl. Here are the other comments. I forgot to submit the review :P

satpy/modifiers/parallax.py Outdated Show resolved Hide resolved
satpy/modifiers/parallax.py Outdated Show resolved Hide resolved
satpy/modifiers/parallax.py Outdated Show resolved Hide resolved
satpy/modifiers/parallax.py Outdated Show resolved Hide resolved
satpy/modifiers/parallax.py Outdated Show resolved Hide resolved
satpy/modifiers/parallax.py Outdated Show resolved Hide resolved
satpy/modifiers/parallax.py Outdated Show resolved Hide resolved
satpy/modifiers/parallax.py Outdated Show resolved Hide resolved
satpy/modifiers/parallax.py Show resolved Hide resolved
satpy/modifiers/parallax.py Outdated Show resolved Hide resolved
Add utility function for parallax displacement, as suggested by @ghiggi.
Rename the function `forward_parallax` to be called
`get_parallax_corrected_lonlats`.
Various smaller edits/changes in documentation and variable names.
Copy link
Member

@djhoese djhoese left a comment

Choose a reason for hiding this comment

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

LGTM

PCW Autumn 2021 automation moved this from Review in progress to Reviewer approved Jul 28, 2022
@djhoese djhoese changed the title Implement parallax correction Add parallax correction via new ParallaxCorrectionModifier Jul 28, 2022
@djhoese djhoese merged commit 8e039d6 into pytroll:main Jul 28, 2022
PCW Autumn 2021 automation moved this from Reviewer approved to Done Jul 28, 2022
PCW Spring 2022 automation moved this from Ready for review to Done Jul 28, 2022
@djhoese
Copy link
Member

djhoese commented Jul 28, 2022

It. is. done. 🎉

@mraspaud
Copy link
Member

Thank you @gerritholl for driving this to its completion, and thanks to everyone involved for taking the time to make this PR better!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement code enhancements, features, improvements
Development

Successfully merging this pull request may close these issues.

Add parallax correction