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

Implement modifier for reducing signal as a function of sunz angle #2556

Merged
merged 8 commits into from
Oct 10, 2023

Conversation

strandgren
Copy link
Collaborator

@strandgren strandgren commented Aug 24, 2023

This PR Implements a new modifier that reduces the signal within a defined sunz angle interval in order to avoid the excessive white glow at the limb in geostationary VIS/NIR imagery. An example on how to define it and use it is shown below:

modifiers:
  sunz_reduced:
    modifier: !!python/name:satpy.modifiers.SunZenithReducer
    correction_limit: 55
    max_sza: 90
    strength: 1.5
    optional_prerequisites:
      - solar_zenith_angle

composites:
   true_color:
    compositor: !!python/name:satpy.composites.SelfSharpenedRGB
    prerequisites:
      - name: vis_06
        modifiers: [sunz_corrected, rayleigh_corrected, sunz_reduced]
      - name: ndvi_hybrid_green
      - name: vis_04
        modifiers: [sunz_corrected, rayleigh_corrected, sunz_reduced]
    standard_name: true_color

The ability to reduce the signal as a function of the sunz angle is already implemented as part of e.g. the SunZenithCorrector modifier which generally works very well for reducing the correction at high angles. The problem occurs when applying Rayleigh correction on imagery where the sunz correction has been reduced at high angles, since pyspectral assumes regular reflectance data normalized by the sun zenith angle when estimating the Rayleigh contribution. This issue is clearly demonstrated in the examples below:

image

It was discussed whether the Rayleigh correction can be adapted to account for this, but after another round of investigation and testing I still did not find a proper way to do this. Hence, I suggest to add the proposed modifier and then when users have had a chance to test it, we can see if we can find a better implementation. A better implementation would mean that the user defines the reduction in the sunz correction and then the Rayleigh correction properly accounts for this (meaning that there would be no need for a third modifier).

The results with the modifier implemented here, following the example recipe above looks like this:
image

Another example with the terminator across the disk, without and with the sunz reduction modifier in this PR:
image

The default values for the reduction (correction_limit: 55, max_sza: 90, strength: 1.5) have been chosen to give an overall good trade-off in having a nice transition into deep-space at the limb and not loosing too much information at the terminator. Nevertheless these values can of course be modified by any user that want to use the modifier.

As can be seen in the last example the modifier remove some information at the terminator. Nevertheless, it shall be noted that:

  • Without the modifier the colors in this area are generally not well represented.

  • The effect get's less evident/relevant when blending the composite with a night-time layer

  • It should be possible to restore some of the information by reducing the Rayleigh correction at high sunz angles.

  • Tests added

  • Fully documented

@strandgren
Copy link
Collaborator Author

Another example for AHI:

image

@codecov
Copy link

codecov bot commented Aug 24, 2023

Codecov Report

Merging #2556 (ba655df) into main (6326b03) will increase coverage by 0.00%.
Report is 43 commits behind head on main.
The diff coverage is 100.00%.

@@           Coverage Diff            @@
##             main    #2556    +/-   ##
========================================
  Coverage   94.91%   94.91%            
========================================
  Files         349      351     +2     
  Lines       51157    51259   +102     
========================================
+ Hits        48558    48655    +97     
- Misses       2599     2604     +5     
Flag Coverage Δ
behaviourtests 4.30% <16.27%> (+<0.01%) ⬆️
unittests 95.52% <100.00%> (-0.01%) ⬇️

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

Files Changed Coverage Δ
satpy/modifiers/__init__.py 100.00% <100.00%> (ø)
satpy/modifiers/angles.py 99.21% <100.00%> (+0.03%) ⬆️
satpy/modifiers/geometry.py 90.32% <100.00%> (+2.56%) ⬆️
satpy/tests/test_modifiers.py 100.00% <100.00%> (ø)

... and 20 files with indirect coverage changes

@coveralls
Copy link

coveralls commented Aug 24, 2023

Pull Request Test Coverage Report for Build 6224673446

  • 43 of 43 (100.0%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.004%) to 95.47%

Totals Coverage Status
Change from base Build 6190601890: 0.004%
Covered Lines: 48768
Relevant Lines: 51082

💛 - Coveralls

@mraspaud
Copy link
Member

mraspaud commented Sep 8, 2023

I'm wondering if changing the sunz cutoff would have a similar effect?

@strandgren
Copy link
Collaborator Author

strandgren commented Sep 8, 2023

I'm wondering if changing the sunz cutoff would have a similar effect?

That's what I try to demonstrate with the four images in the PR description. It works well as long as you don't apply any Rayleigh correction, but as soon as the Rayleigh correction is applied, you get issues at the limb since (I believe) pyspectral assumes reflectance data where no such reduction has been applied.

@mraspaud
Copy link
Member

mraspaud commented Sep 8, 2023

That's what I try to demonstrate with the four images in the PR description. It works well as long as you don't apply any Rayleigh correction, but as soon as the Rayleigh correction is applied, you get issues at the limb since (I believe) pyspectral assumes reflectance data where no such reduction has been applied.

Ah, indeed, sorry.

Comment on lines 117 to 118
smaller the contribution from the nir channel will be, following a liner (default) or non-linear relationship
between the two ranges `[ndvi_min, ndvi_max]` and `limits`.
Copy link
Member

Choose a reason for hiding this comment

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

This is from another PR, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

hmm yes.. Seems like I may have branched out from the branch used for the other PR.. May I ask what's the best way to deal with this? Reset this branch or leave it as if for now and then pull in master and modify accordingly once the other PR has been merged?

Copy link
Member

Choose a reason for hiding this comment

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

I don't know if it's the right way, but I'd just remove here the code that is implemented in the other PR, and commit that. And then later merge the main branch.

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, but I think there are two PRs mixed in here.

@strandgren
Copy link
Collaborator Author

LGTM, but I think there are two PRs mixed in here.

I think this should be resolved now :)

@mraspaud mraspaud added enhancement code enhancements, features, improvements component:compositors labels Oct 10, 2023
@mraspaud mraspaud merged commit c252ed7 into pytroll:main Oct 10, 2023
15 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:compositors enhancement code enhancements, features, improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants