Skip to content

Refactor code for Sun zenith angle corrections and change to effective_solar_pathlength_corrected instead of sunz_corrected for built-in RGB recipes#3397

Open
strandgren wants to merge 49 commits into
pytroll:mainfrom
strandgren:refactor_solar_zenith_angle_correction_methods
Open

Refactor code for Sun zenith angle corrections and change to effective_solar_pathlength_corrected instead of sunz_corrected for built-in RGB recipes#3397
strandgren wants to merge 49 commits into
pytroll:mainfrom
strandgren:refactor_solar_zenith_angle_correction_methods

Conversation

@strandgren
Copy link
Copy Markdown
Collaborator

@strandgren strandgren commented May 20, 2026

This PR aims to clarify the code used for the Sun zenith angle corrections in satpy, including improving documentation and log messages to make it more transparent to the user. Furthermore, the default behavior of the SunZenithCorrector/sunz_corrected modifier is changed to remove the default reduction at high angles in order to compute the true reflectance. Finally, the built-in recipes are modified to improve imagery and comply with the recommendation from the WMO RGB workshop in Norrkoeping in April 2025.

Background
In satpy we have two corrections for the Solar zenith angle/Solar path length available, the standard SunZenithCorrector which applies the 1/cos(sunz) correction and the EffectiveSolarPathLengthCorrector which applies the parameterization proposed by Li and Shibata (2006).

There is also a default reduction of the correction starting at a given Sun zenith angle (88 degrees). This is intended for (RGB) imagery to avoid overcorrection at very large angles. However, this is not desirable for quantitative or scientific use of the data, where the standard 1/cos(sunz) should be used as is in order to compute the true reflectance. This functionality for capping/reduction has also been implemented in the EffectiveSolarPathLengthCorrector (duplicate code), but as shown in #3096 this reduction doesn't have any added value for the Li and Shibata parameterization, which works best as is, since it already accounts for the overcorrection at higher angles. For this reason, it was also agreed at the WMO RGB workshop in Spring 2025, that the Li and Shibata parameterization should be the recommended method for normalizing the data for the Solar zenith angle (without any capping or reduction).

Furthermore, some datasets come with the 1/cos(sunz) already applied, in which case the sunz_corrected modifier is attached to the dataset. However, this is not accurate since the default satpy sunz_corrected modifier is not only the simple 1/cos(sunz), but also includes the reduction of the correction at higher Solar zenith angels. Therefore it would also be good if the default sunz_corrected would rather be the simple 1/cos(sunz) corrected, which would then be in-line with these datasets and also allow users in general to compute the true reflectance - this becomes even more relevant with #3292 where we clearly separate between "reflectance" data normalized by the Solar zenith angel and not.


Changes in this PR
In response to the points above this PR includes the following changes:

Python code:

  • Remove the capping/reduction of the EffectiveSolarPathLengthCorrector modifier. The paramters for this are still kept as valid input to avoid run-time errors if provided, but have no effect. A warning is issues if provided.
  • Move the underlying code for EffectiveSolarPathLengthCorrector to the same place as the other Solar zenith angle correction methods (./satpy/satpy/modifiers/angles.py)
  • Refactor SunZenithCorrector and underlying methods to make the functionality more clear.
  • Change the default behavior of SunZenithCorrector to compute the true reflectance without any reduction of the the correction. Kept as is for now, to be changed for satpy v1.0.
  • Clarify documentation for SunZenithCorrector and EffectiveSolarPathLengthCorrector, also highlighting the differences between the two and clarify that the former can be used to compute the true reflectance, whereas the latter is tailored for imagery.
  • Modify methods to be dask-compatible and remove wrapping using map_blocks
  • Add more log-messages and warnings depending on the use of the two corrections.
  • Extend test coverage, especially since some parts were previously not tested

yaml-recipes

  • Change pre-configured (RGB) imagery recipes to use the effective_solar_pathlength_corrected modifier instead of sunz_corrected for the solar zenith angle correction. As demonstrated in Standardize the correction for atmospheric path length for solar channels #3096 this leads to better imagery and was also suggested as recommendation at WMO RGB workshop in spring 2025.
  • Change pre-configured Rayleigh correction modifiers for (RGB) imagery recipes to use the effective_solar_pathlength_corrected modifier instead of sunz_corrected for the solar zenith angle correction of the red band used for reduced correction over clouds. This is done since sunz_corrected is changing to become the simple 1/cos(sunz) correction which would not work well for imagery purposes since the reflectance of the red band would become 0 for angles larger than 90 degrees and thus break the Rayleigh correction. The Rayleigh correction reduction over clouds would also not work well close to 90 degrees, since also clear-sky becomes very bright at high sun zenith angles with the 1/cos(sunz) correction.
  • Remove sunz_reduced modifier for FCI true_color, cloud_phase and cloud_type RGBs to be in-line with other sensors and recommendation from RGB workshop. Additional true_color_sunz_reduced composite is added to still support the generation of this RGB with a smoother transition into deep-space.

Special cases
Some readers provide the reflectance data with the 1/cos(sunz) correction already applied, in which case the sunz_corrected modifier is already attached to the dataset. This is the case for the following readers:

  • meris_nc_sen3
  • msi_safe_l2a
  • olci_l2
  • viirs_sdr
  • clavrx

For meris_nc_sen3, olci_l2 and clavrx there are no pre-configured RGB recipes, so the changes in this PR are not relevant for these data. For msi_safe_l2a the data have the dedicated modifier esa_sunz_corrected attached to them, which is also used for the corresponding RGB recipes. Hence, the changes in the PR are not relevant for these data either.

The only relevant one is VIIRS (SDR). Since the VIIRS SDR data come with the Solar zenith angle correction already applied, the dataset gets the sunz_corrected modifier attached. However, VIIRS L1B data come without the 1/cos(sunz) correction and therefore no sunz_corrected modifier is attached to the dataset. For this reason, we can use the EffectiveSolarPathLengthCorrector modifier for the recipes, but we have to name it sunz_corrected, in order for satpy to understand that this is equivalent to the sunz correction already in the SDR data in the sense that no further correction should be applied.

A better and more generic solution would be to modify the dependency tree to know that if the user requests a dataset with the effective_solar_pathlength_corrected modifier, but only finds a DataID with the modifier sunz_corrected, it would be regarded as equivalent and a substitute for the effective_solar_pathlength_corrected modifier. However, such a solution is outside the scope of this PR.

After the monthly meeting on June 2, it was agreed to use the regular effective_solar_pathlength_corrected modifier also for VIIRS data. Currently this won't work with VIIRS SDR data, since satpy will fail to find the "base" version of the requested without modifiers. It was agreed that this should be solved by adding the unnormalized_reflectance calibration level to the viirs_sdr reader once #3292 is merged, see details in #3411. We then just need to make sure that the VIIRS composites specify the expected calibration level in the composite and/or modifier recipes.


Changes for users and backwards incompatibility

  • For the built-in satpy recipes the changes are generally minor and to the better with smoother imagery close the the terminator. For example the bright line along the terminator is no longer there which was caused by the very sharp peak of the sunz_corrected modifier at 88 degrees. See Standardize the correction for atmospheric path length for solar channels #3096 and comments in the PR for examples.
  • For FCI users, the removal of the sunz_reduced modifier leads to significantly more data being retained/visible close to the terminator. see examples below.
  • The reduction of the effective_solar_pathlength_corrected modifier has been removed as it's not needed, given that the Li and Shibata parameterization already deals with the over-correction at high solar zenith angels. If the reduction parameters are still provided they will be accepted but ignored and a warning is raised.
  • VIIRS composites using the effective_solar_pathlength_corrected modifier will not be possible with VIIRS SDR data until Add unnormalized_reflectance to VIIRS SDR (viirs_sdr) reader #3411 has been fixed.
  • The default reduction of the sunz_corrected has been disabled (but still with support to apply it if needed). Hence, the sunz_corrected modifier will compute the true reflectance if applied with it's new default configuration. This will impact local composites which use the upstream sunz_corrected modifiers defined in satpy (except for VIIRS which is treated differently given the differences between the L1B and SDR data). The result will be very bright imagery close to 90 degrees and no data beyond 90 degrees. A warning has been added when using these modifiers to inform users on this change and how to fix it.
  • Similarly, if users have defined there own local sunz_corrected modifier with default configuration (i.e. no correction_limit or max_sza), the imagery will change close to the terminator (again becoming very bright close to 90 degrees and black beyond).

Following the monthly meeting on June 2, this PR has been changed such that there are no changes to the default values in SunZenithCorrector or corresponding pre-configured sunz_corrected modifiers in satpy at this point. Instead we will change this in v1.0 (#3412) and a warning is therefore now raised about this upcoming change if the default values of SunZenithCorrector are used.



AI was used to support this PR, mainly for refactoring tests and writing doc-strings

@strandgren strandgren self-assigned this May 20, 2026
@strandgren strandgren added refactor backwards-incompatibility Causes backwards incompatibility or introduces a deprecation PCW Pytroll Contributors' Week cleanup Code cleanup but otherwise no change in functionality component:modifiers labels May 20, 2026
@strandgren strandgren moved this from Backlog to In progress in PCW Spring 2026 May 20, 2026
@djhoese
Copy link
Copy Markdown
Member

djhoese commented May 20, 2026

In that case, we could change all composites in Satpy to use EffectiveSolarPathLengthCorrector instead (already proposed in #3096). However, that would lead to backward incompatibility and change of behavior for users having defined SunZenithCorrector with default configuration in the local configurations (local configurations specifying correction_limit and max_sza would still work as before).

I think in general I'm not against this PR "in spirit". The one thing I had trouble with when trying to switch to effective path length as described in your point above is that for some readers the / cos(SZA) is already applied as part of the file creation. For example, VIIRS SDRs. So for most instrument-specific composite definitions we could define RGBs with a the effective path length modifier as part of the prerequisites and users could request the "sunz_corrected" if they wanted to "do science".

However, I think this means that for VIIRS composites we'd have to redefine the sunz_corrected modifier to be the effective path length and use it for all RGBs. This way VIIRS SDRs don't get any extra correction and the RGBs can be made, but then for VIIRS L1Bs (where no / cos(SZA) is pre-applied) they would get the effective solar pathlength correction and then RGBs could be made. This does mean RGBs would look different between VIIRS SDR and L1B inputs though.

@simonrp84
Copy link
Copy Markdown
Member

You could always reverse the cos(sza) and then apply the effective path length...

@djhoese
Copy link
Copy Markdown
Member

djhoese commented May 20, 2026

🤔 I'd prefer not to.

@strandgren
Copy link
Copy Markdown
Collaborator Author

@djhoese I understand the concern, but if I understand the comment correctly I don't think the proposed changes would be a problem and no significant change to the current behavior.

Now the default sunz_corrected modified already reduces the correction between 88-95 degrees in order to avoid over-correction and get better looking imagery. Hence, the standard sunz_corrected does not compute the real reflectance at high sunz angles and tus dehaves differently compared to the correction already applied in the VIIRS SDR data. This reduction is a rather crude alternative to the Li and Shibata parameterization, which I tried to demonstrate in #3096. What this means:

  1. If you derive RGBs from VIIRS SRD data with the pure 1/cos(sunz) correction already applied in the data, they will already look different compared to RGBs derived from VIIRS L1B data where the /cos(sunz) correction including default reduction is applied in satpy.
  2. If preferred, you will still be able to preserve the current default behavior of sunz_corrected by defining it like this:
  sunz_corrected_legacy:
    modifier: !!python/name:satpy.modifiers.SunZenithCorrector
    correction_limit: 88
    max_sza: 95
    prerequisites:
    - name: solar_zenith_angle
      resolution: 742

Does that make sense and sound ok for you you, or did I miss something else?

I'll try to implement my suggested changes today such that it's more clear what I propose, and then we can see i the review process if this is acceptable or if I should revert something.

@djhoese
Copy link
Copy Markdown
Member

djhoese commented May 21, 2026

What you say makes sense but I can't tell if you're saying we would still need to do what I mentioned. That is, VIIRS composites would need to use "sunz_corrected" for RGBs but either have to:

  1. Stick with regular sunz correction with or without caps.
  2. Switch to path length to match the rest of satpy but still call the modifier "sunz_corrected" so that VIIRS SDRs could still be used for RGBs.

- Fix bug to make sure that custom sza values are used when actually intended and pass as optional_dataset instead of as a projectable.
- Use different sza values in order to be able to properly test the available options concerning correction_limit and max_sza.
- Add tests for more combinations of correction_limit and max_sza
…sed as expected. Fix expected values to pass updated values of sunz_sza.
@strandgren
Copy link
Copy Markdown
Collaborator Author

What you say makes sense but I can't tell if you're saying we would still need to do what I mentioned. That is, VIIRS composites would need to use "sunz_corrected" for RGBs but either have to:

  1. Stick with regular sunz correction with or without caps.
  2. Switch to path length to match the rest of satpy but still call the modifier "sunz_corrected" so that VIIRS SDRs could still be used for RGBs.

Indeed, I think that's needed unless the dependency tree is modified. In the end I went for Option 2 above, which I think is the best solution for now.

@strandgren strandgren marked this pull request as draft May 28, 2026 09:26
@strandgren strandgren marked this pull request as ready for review May 28, 2026 09:27
@strandgren
Copy link
Copy Markdown
Collaborator Author

This PR is now ready for review 🙂

Two important points/questions at this point:

  1. Now I have changed the default behavior of sunz_corrected to compute the true reflectance, i.e. applying the simple 1/cos(sunz) without any reduction. I think this is a relevant change also in view of Introduce unnormalized_reflectance calibration level #3292. However, for users that use these pre-configured modifiers in the local RGB recipes, the imagery will change in the terminator area (>88 degrees) and become very bright close to 90 degrees and black beyond. I have added a warning to all those modifiers about this change and how to fix it. Still I don't know if this is an acceptable change without prior warning? Initially I had kept the old reduction of these modifiers in order to be keep the current behavior (including a warning about a future change), but I wasn't sure if it would be possible to change the actual behavior in a later version, since it would probably be >v1.0?

  2. @gerritholl Any chance the image comparison tool could run on this PR? I have tested the changes with real data, but it would be good to have an independent test as well if possible

@strandgren
Copy link
Copy Markdown
Collaborator Author

strandgren commented May 28, 2026

Here are some FCI and SEVIRI images (using built-in recipes) in support of the changes in this PR, temporarily also available in a PDF here: https://sftp.eumetsat.int/public/file/abx2zrqqyuc6koinualwra/satpy_pr3397.pdf.

image image image image image image

…loud_type_sunz_reduced and cloud_phase_sunz_reduced recipes given clear benefit of more data being retained without it, without obvious artefacts, as well as alternative tuning by CHMI to reduce saturation.
Copy link
Copy Markdown
Member

@ameraner ameraner left a comment

Choose a reason for hiding this comment

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

Nice work, thanks! The only thing I find a bit of a pity is that the default true_color now gets back the reddish clouds at the terminator. But I can see the point of being consistent with all the other recipes and wanting to keep as much daytime as possible, and there's now also true_color_sunz_reduced as an alternative option that is close to the old appearance.

Just a suggestion inline.

Comment thread satpy/etc/composites/fci.yaml Outdated
Comment thread doc/source/modifiers.rst Outdated
strandgren and others added 2 commits May 28, 2026 15:14
Co-authored-by: Andrea Meraner <49722768+ameraner@users.noreply.github.com>
@strandgren
Copy link
Copy Markdown
Collaborator Author

The only thing I find a bit of a pity is that the default true_color now gets back the reddish clouds at the terminator. But I can see the point of being consistent with all the other recipes and wanting to keep as much daytime as possible, and there's now also true_color_sunz_reduced as an alternative option that is close to the old appearance.

yeah, I thought keeping as much data as possible and staying consistent across RGBs and sensors was more deisreble in the end. Seeing the reddish artefacts can also be a consistent reminder for us to improve the Rayleigh correction at these high angles 😃

And indeed, we also have the true_color_sunz_reduced now, which is very similar to the current behavior of true_color. And using true_color in this PR, but masking out the night-time part using the DayNightCompositor, we can get quite a good compromise. Here is a comparison of the four variants:

image

That being said, if others area also against this change, I can revert it for FCI true_color 🙂

@strandgren
Copy link
Copy Markdown
Collaborator Author

  1. Now I have changed the default behavior of sunz_corrected to compute the true reflectance, i.e. applying the simple 1/cos(sunz) without any reduction. I think this is a relevant change also in view of Introduce unnormalized_reflectance calibration level #3292. However, for users that use these pre-configured modifiers in the local RGB recipes, the imagery will change in the terminator area (>88 degrees) and become very bright close to 90 degrees and black beyond. I have added a warning to all those modifiers about this change and how to fix it. Still I don't know if this is an acceptable change without prior warning? Initially I had kept the old reduction of these modifiers in order to be keep the current behavior (including a warning about a future change), but I wasn't sure if it would be possible to change the actual behavior in a later version, since it would probably be >v1.0?

As per our discussion in the monthly meeting I have now modified this PR to do the following:

  • Keep the current default values/reduction for SunZenithCorrector/sunz_corrected and add warning that these will change in v1.0. That way there are no changes concerning sunz_corrected (also not local versions of it) with this PR. We will then change the default behaviour in Satpy v1.0 in order to compute the true reflectance (Change default behaviour of SunZenithCorrector/sunz_corrected #3412).
  • Change etc/composites/viirs.yaml to use the plain EffectiveSolarPathLengthCorrector/effective_solar_pathlength_corrected modifier for built-in composites. This won't work with VIIRS SDR data until Add unnormalized_reflectance to VIIRS SDR (viirs_sdr) reader #3411 is solved and we define the expected calibration level (unnormalized_reflectance) in the composite/modifier recipes.

@djhoese
Copy link
Copy Markdown
Member

djhoese commented Jun 3, 2026

Could we add an option to satpy.config (and document it) that is used inside the SunZenithCorrector class and controls whether or not the existing reduction is done versus no reduction? You might have to change the kwarg defaults for now to be None, check if they are None and depending on the satpy.config option do either the current default limits or no limits. Or if None is the current "no correction" option then maybe a sentinel like "__default__" string. That way, if a user specifies the options in the YAML they will still be used.

If this is done, would you also need to update some of the FCI "uncorrecting" logic? I think it was FCI or maybe AHI?

@strandgren
Copy link
Copy Markdown
Collaborator Author

Could we add an option to satpy.config (and document it) that is used inside the SunZenithCorrector class and controls whether or not the existing reduction is done versus no reduction? You might have to change the kwarg defaults for now to be None, check if they are None and depending on the satpy.config option do either the current default limits or no limits. Or if None is the current "no correction" option then maybe a sentinel like "__default__" string. That way, if a user specifies the options in the YAML they will still be used.

Working with the satpy.config is new territory for me, but I will have a look. So the benefit is that you can change the behavior of all of satpy without having to change any code or roll-back to another version? In general I don't see a huge advantage, since we want to get away from the current default correction, but I guess it helps users during migration. Or what is the main argument for this?

If this is done, would you also need to update some of the FCI "uncorrecting" logic? I think it was FCI or maybe AHI?

For FCI (and all other instruments/recipes) there wouldn't be any issue since we no longer use the sunz_corrected modifier and it's reduction in the built-in recipes with this PR since we change to effective_solar_pathlength_corrected. Or what do you mean with "uncorrecting"?

@djhoese
Copy link
Copy Markdown
Member

djhoese commented Jun 3, 2026

RE uncorrecting: I have some memory of you coming up with some correction "reduction" when you were working on geo_color in Satpy to make the sunz correction not so aggressive. Or something like that.

RE satpy.config: The idea would be to implement a "feature flag" essentially. This is kind of what I talked about in the meeting. The idea is that someone using satpy pre-1.0 could use Satpy as-is and get the same output as before, but when they are ready they can set satpy.config.set(sunz_legacy_defaults=False) and then suddenly they get the changes they'll see in 1.0. Then we deprecate that config argument for Satpy 1.1 for example which is a feature the donfig library provides. At least as the maintainer of that package I'm pretty sure that functionality exists. This is similar to what other 1.0 features are doing too (WMO instrument renaming, calibration levels, etc).

It would come down to adding something to satpy._config for the new value, documenting it in the sphinx config.rst file, and then doing an if statement in the corrector like:

if some_limit == "__default__":
    if satpy.config.get("sunz_legacy_defaults", True):
        some_limit = 88
    else:
        # no correction

With maybe a warning added to the True case.

@strandgren
Copy link
Copy Markdown
Collaborator Author

Alright, I will look into that, thanks for elaborating 🙂

Regarding the "uncorrection", we worked on it for FCI indeed and we call it sunz_reduced. In the end I removed it from the default FCI recipes here and instead added it in the new composite called true_color_sunz_reduced . The thing is that with that reducer you can get very nice looking imagery with a smooth transition into deep space, but with the caveat that you loose data at the terminator, which is especially evident when the terminator runs across the disk. And at this point we figured that it's more important to get the most useful imagery, and of secondary importance to get the most beautiful 😄

But again, the changes to the the default reduction of the sunz_corrected modifier doesn't have a big impact on that anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backwards-incompatibility Causes backwards incompatibility or introduces a deprecation cleanup Code cleanup but otherwise no change in functionality component:compositors component:modifiers PCW Pytroll Contributors' Week refactor

Projects

Status: In review

Development

Successfully merging this pull request may close these issues.

Refactor code for solar zenith angle correction modifiers

4 participants