Fix CREFL using incorrect coefficients for MODIS

[djhoese]

While working on some Polar2Grid 3.0 validation and comparison, I noticed that the MODIS true_color composites with CREFL rayleigh correction looked different than in past versions. The past versions use the official C version of the CREFL/CVIIRS code. After some debugging it was clear the Satpy CREFL implementation was different. What was used in Satpy was only the VIIRS and ABI coefficients worked on by @ralphk11. The other coefficients needed for MODIS were hidden behind a global if statement.

I think this PR fixes everything I was concerned about. It is a little uglier than I was hoping for now that I have to pass sensor_name further into the atmosphere code, but the alternative was some duplicated code. I'm satisfied that the MODIS tests are the only ones that needed modification.

CC @wroberts4 who originally tried to merge all of the crefl algorithms together.

Edit: To clarify, the MODIS processing was incorrectly using VIIRS coefficients and code paths rather than the official MODIS ones.

• Closes #xxxx
• Tests added
• Fully documented

[codecov]

Codecov Report

Merging #2084 (6dbb75e) into main (d4c4332) will increase coverage by 0.02%.
The diff coverage is 94.60%.

@@            Coverage Diff             @@
##             main    #2084      +/-   ##
==========================================
+ Coverage   93.86%   93.89%   +0.02%     
==========================================
  Files         283      283              
  Lines       42379    42500     +121     
==========================================
+ Hits        39781    39905     +124     
+ Misses       2598     2595       -3     

Flag	Coverage Δ
behaviourtests	4.71% <0.41%> (-0.02%)	⬇️
unittests	94.45% <94.60%> (+0.02%)	⬆️

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

Impacted Files	Coverage Δ
satpy/modifiers/_crefl_utils.py	95.09% <93.95%> (+3.04%)	⬆️
satpy/modifiers/_crefl.py	92.40% <100.00%> (-0.62%)	⬇️
satpy/tests/modifier_tests/test_crefl.py	100.00% <100.00%> (+0.51%)	⬆️
satpy/tests/test_crefl_utils.py	100.00% <100.00%> (ø)
satpy/readers/seviri_l1b_hrit.py	90.14% <0.00%> (ø)
satpy/tests/reader_tests/test_utils.py	100.00% <0.00%> (ø)
satpy/tests/reader_tests/test_viirs_l1b.py	97.26% <0.00%> (+0.06%)	⬆️
satpy/readers/utils.py	92.34% <0.00%> (+0.07%)	⬆️
satpy/tests/reader_tests/test_hrit_base.py	98.21% <0.00%> (+0.15%)	⬆️
satpy/readers/viirs_l1b.py	95.77% <0.00%> (+0.60%)	⬆️
... and 1 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d4c4332...6dbb75e. Read the comment docs.

[coveralls]

Coverage increased (+0.02%) to 94.393% when pulling 6dbb75e on djhoese:bugfix-crefl-new-coeffs into d4c4332 on pytroll:main.

[djhoese]

Old:

New:

[mraspaud]

Good that the coeffs are not mixed up anymore. However, we should have a nicer way to handle the coefficients for the different sensors. Hardcoded index offsets ?!?

[simonrp84]

Not directly related to this PR itself, but is there some documentation / a paper / something that describes CREFL that we can add to the docstring? Would be good to have something more insightful than "Ralph did it" if possible ;-)

[djhoese]

@simonrp84 The CREFL software is the NASA MODIS CREFL package:
https://directreadout.sci.gsfc.nasa.gov/?id=dspContent&cid=91

And the NASA VIIRS CVIIRS SPA software (link requires login so not copying here).

The stuff by Ralph is nothing official as far as I know. @ralphk11 can comment. I'm not sure if it has been published. I can, should, and will document more about this being from the NASA algorithms.

As far as you question on slack:

Why does ABI use a different set of atmospheric variables to MODIS/VIIRS?

Ralph said this about the changes as a whole:

The main changes that I made with Min Oo was that we calculated new coefficients for AHI based upon a radiative transfer model that didn’t assume the atmosphere was infinite plane-parallel. It takes into account the sphericity of the earth, which is important for the geostationary sensors at medium view angles. For VIIRS and MODIS this isn’t necessary and a plane-parallel approximation is fine.

[djhoese]

@mraspaud I've refactored the parts of the code I could. I wanted to avoid passing instance methods to map_blocks because I don't think they serialize properly on a distributed scheduler. Whether or not my refactor is an overall improvement, I'm not sure. I think the code felt cleaner the more I worked with it, but that could be because I was getting used to it.

@simonrp84 @ralphk11 I've added more information to the docstring including linking to the NASA DRL pages for CREFL and CVIIRS. This whole module has an _ prefix so it is not included in the Satpy API docs by default so no one will see this on the Satpy documentation site.

[ralphk11]

@simonrp84 The CREFL software is the NASA MODIS CREFL package: https://directreadout.sci.gsfc.nasa.gov/?id=dspContent&cid=91

And the NASA VIIRS CVIIRS SPA software (link requires login so not copying here).

The stuff by Ralph is nothing official as far as I know. @ralphk11 can comment. I'm not sure if it has been published. I can, should, and will document more about this being from the NASA algorithms.

As far as you question on slack:

Why does ABI use a different set of atmospheric variables to MODIS/VIIRS?

Ralph said this about the changes as a whole:

The main changes that I made with Min Oo was that we calculated new coefficients for AHI based upon a radiative transfer model that didn’t assume the atmosphere was infinite plane-parallel. It takes into account the sphericity of the earth, which is important for the geostationary sensors at medium view angles. For VIIRS and MODIS this isn’t necessary and a plane-parallel approximation is fine.

For reference the original CREFL code is similar to what is described in appendix A1 (page 74) of the ATBD for the MODIS MOD04/MYD04 data product. Note that for some reason the linked filename says MOD02(??) https://modis.gsfc.nasa.gov/data/atbd/atbd_mod02.pdf.

The AHI/ABI implementation is based on the MODIS collection 6 algorithm, where a spherical-shell atmosphere was assumed rather than a plane-parallel. See appendix A in: "The Collection 6 MODIS aerosol products over land and ocean" Atmos. Meas. Tech., 6, 2989–3034, 2013 www.atmos-meas-tech.net/6/2989/2013/ doi:10.5194/amt-6-2989-2013

[djhoese]

Added. Thanks @ralphk11

[djhoese]

Unstable environment failures are from this PR in xarray which should be fixed shortly (hopefully): pydata/xarray#6471

[mraspaud]

Thanks for the refactoring, it looks definitely better. We could surely go further with it, but that could be in another future PR if you don't feel like going further on this right now. I have one comment inline which I think needs addressing, and then there are quite a few places where coverage is allegedly missing...

[mraspaud]

Looks good, just a few comments.

[djhoese]

Surprisingly the precision differences between my system and CI seems decent (1e-6). I've updated the tests to be a little friendly.

[djhoese]

Whoa, updated locally and it looks like I'm seeing similar failures. A new version of numpy or something must be changing results.

[djhoese]

I just pushed a small commit that removed the computing_meta keyword argument from dask. This is unnecessary now that I pass meta= to the map_blocks call.

[mraspaud]

LGTM!