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 configurable parameters to solar zenith correctors #545

Merged
merged 8 commits into from
Dec 14, 2018

Conversation

djhoese
Copy link
Member

@djhoese djhoese commented Dec 12, 2018

Main Purpose

This PR adds YAML-configurable parameters to the two solar-zenith correctors/modifiers. The three parameters that are now available in both are:

  • correction_limit (float): Maximum solar zenith angle to apply the correction in degrees. Pixels beyond this limit have a constant correction applied. Default 88.
  • min_sza (float): Minimum solar zenith angle in degrees that is considered valid and correctable. Default 0.0.
  • max_sza (float): Maximum solar zenith angle in degrees that is considered valid and correctable. Default 90.0.

Behavior Change

Currently, this PR has changed the default values for these parameters based on some tests I did with ABI natural_color and true_color RGBs. Previously, the equivalent value for max_sza was None due to some undesired (in my opinion) usage of the fill values in the correction code. The way the corrections were previously set up is that for data past 88 degrees a constant correction is applied to the data regardless of SZA. So for SZA 0 to 88 the correction was applied normally, for 88-90+ a constant correction was being applied, even if SZA was NaN. In this PR I've changed it to be corrected from 0 to 88, constant from 88-90 (max_sza), and NaN after that. This allows satpy and the user to say "anything past this SZA isn't useful data". The main reason for this change was due to noticing that ABI natural color RGBs were having night time pixels "corrected", increasing the "useless" data values.

Note that the user can get the same behavior as before by specifying max_sza: !!null in the YAML for the sunz_corrected.

I am ok not including this behavior change (I'd still like it) because I can have it customized in my own projects (Polar2Grid/Geo2Grid).

Examples

NOTE: The default True Color enhancement sets all negative reflectances to NaN/transparent.

GOES-16 ABI True Color - Old

image

GOES-16 ABI Natural Color - Old

image

GOES-16 ABI True Color - 0 to 88 to 90

image

GOES-16 ABI Natural Color - 0 to 88 to 90

image

GOES-16 ABI True Color - 0 to 88 to 92

image

GOES-16 ABI Natural Color - 0 to 88 to 92

image

GOES-16 ABI True Color - 0 to 88 to 95

Same as 0 to 88 to 92 image.

GOES-16 ABI Natural Color - 0 to 88 to 95

image

Miscellaneous

I originally started working on this because the natural color image shows bright green and red pixels in the night time data. I was able to prove that negative reflectances were part of the problem and were being made worse by the SZA corrector and by the sharpening code. However, it didn't get rid of all of it and cutting off the SZA correction seemed to make more sense.

  • Tests added
  • Tests passed
  • Passes git diff origin/master -- "*py" | flake8 --diff
  • Fully documented

@djhoese djhoese added enhancement code enhancements, features, improvements component:compositors labels Dec 12, 2018
@djhoese djhoese self-assigned this Dec 12, 2018
@codecov
Copy link

codecov bot commented Dec 12, 2018

Codecov Report

Merging #545 into master will increase coverage by 0.71%.
The diff coverage is 78.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #545      +/-   ##
==========================================
+ Coverage   75.97%   76.69%   +0.71%     
==========================================
  Files         136      136              
  Lines       18810    18961     +151     
==========================================
+ Hits        14291    14542     +251     
+ Misses       4519     4419     -100
Impacted Files Coverage Δ
satpy/tests/compositor_tests/__init__.py 99.05% <100%> (+0.09%) ⬆️
satpy/composites/viirs.py 83.59% <100%> (ø) ⬆️
satpy/utils.py 63.7% <48%> (+0.99%) ⬆️
satpy/composites/__init__.py 63.34% <85%> (+3.91%) ⬆️
satpy/readers/nc_nwcsaf.py 35.84% <0%> (-0.28%) ⬇️
satpy/readers/msg_base.py 97.1% <0%> (+0.13%) ⬆️
satpy/tests/writer_tests/test_cf.py 98.21% <0%> (+0.31%) ⬆️
satpy/writers/cf_writer.py 68.84% <0%> (+0.65%) ⬆️
satpy/tests/reader_tests/test_native_msg.py 95.34% <0%> (+20.34%) ⬆️
... 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 56dbddd...9996602. Read the comment docs.

@djhoese
Copy link
Member Author

djhoese commented Dec 13, 2018

I made some images with the DayNightCompositor and an ABI true_color and I think this may need to be changed to 92 to get a small transition between the two images.

@djhoese
Copy link
Member Author

djhoese commented Dec 13, 2018

Just so it is documented I'm reiterating what was discussed on slack here. @mraspaud and @pnuu thought it would be better for the "night" data to be set to 0 rather than NaNs. This serves my project just as well so I am ok with it. Martin also said that 95 degrees might be the best cutoff.

However, the resulting natural color image for this looks...ugly. I am going to try testing gradual dropoffs on the correction so that the data goes to 0 on the edge of the image. I'll have examples later.

@djhoese
Copy link
Member Author

djhoese commented Dec 13, 2018

The following natural color images were created with gradual SZA corrections between 88 and 95 degrees. I tried a linear one but it didn't do a good enough job of cutting off the noise in the night data so I'm not including it here. The "Natural Log (before inversion)" code is what is currently committed and what I think looks the best. Due to the angle limits these different methods have little to no effect on the true color images.

Square root

image

Square root (before inversion)

image

Natural Log

image

Natural Log (before inversion)

image

from satpy.composites import SunZenithCorrector
comp = SunZenithCorrector(name='sza_test', modifiers=tuple())
res = comp((self.ds1,), test_attr='test')
np.testing.assert_allclose(res.values, np.array([[22.401667, 22.31777 ], [22.437503, 22.353533]]))
Copy link
Contributor

Choose a reason for hiding this comment

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

E202 whitespace before ']'

from satpy.composites import SunZenithCorrector
comp = SunZenithCorrector(name='sza_test', modifiers=tuple())
res = comp((self.ds1, self.sza), test_attr='test')
np.testing.assert_allclose(res.values, np.array([[22.401667, 22.31777 ], [22.437503, 22.353533]]))
Copy link
Contributor

Choose a reason for hiding this comment

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

E202 whitespace before ']'

# Conflicts:
#	satpy/composites/__init__.py
#	satpy/tests/compositor_tests/__init__.py
@coveralls
Copy link

coveralls commented Dec 14, 2018

Coverage Status

Coverage increased (+0.7%) to 76.694% when pulling 9996602 on djhoese:feature-sza-limits into 56dbddd on pytroll:master.

@mraspaud
Copy link
Member

My favourite is natural log before inversion

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.

Looks good, just a couple of comments.

satpy/etc/enhancements/generic.yaml Outdated Show resolved Hide resolved
satpy/utils.py Show resolved Hide resolved
satpy/composites/__init__.py Outdated Show resolved Hide resolved
@djhoese
Copy link
Member Author

djhoese commented Dec 14, 2018

I'm not simply a terrible person. Martin and I did discuss this on slack so even though he hasn't officially approved this PR he said it was ok for me to merge this.

@djhoese djhoese merged commit 49027e8 into pytroll:master Dec 14, 2018
@djhoese djhoese deleted the feature-sza-limits branch December 14, 2018 17:40
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