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

Revert np.nan_to_num usage on rayleigh correction array #159

Merged
merged 2 commits into from
Oct 10, 2022

Conversation

djhoese
Copy link
Member

@djhoese djhoese commented Oct 6, 2022

As discussed on slack, the usage of np.nan_to_num that is being changed in this PR was probably originally added with the idea that "if the correction calculations went wrong then we don't want to mask pixels, we just want to not correct them". The problem with this logic in some recent real world cases is that it is more "jarring" to see uncorrected pixels next to corrected pixels instead of black/transparent/masked pixels. See this MERSI-2 case where the angles at 1000m resolution do not go as far west as the band data pixels at 250m (for some reason) and this results in a 0 correction for these 5 or so pixels on the left side of the swath:

image

If we use this PR's changes (and the way it was before PR #140) then these pixels are simply masked away as invalid pixels:

image

Note: The gray background in the first image was me playing around with the fill value.

I will work on making an issue to further investigate np.nan_to_num's usage in Rayleigh correction at terminator regions.

  • Closes #xxxx
  • Tests added
  • Tests passed: Passes pytest pyspectral
  • Passes flake8 pyspectral
  • Fully documented

@codecov
Copy link

codecov bot commented Oct 6, 2022

Codecov Report

Merging #159 (538df91) into main (fd07153) will decrease coverage by 0.04%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #159      +/-   ##
==========================================
- Coverage   88.03%   87.98%   -0.05%     
==========================================
  Files          22       22              
  Lines        2473     2464       -9     
==========================================
- Hits         2177     2168       -9     
  Misses        296      296              
Flag Coverage Δ
unittests 87.98% <100.00%> (-0.05%) ⬇️

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

Impacted Files Coverage Δ
pyspectral/rayleigh.py 95.48% <100.00%> (ø)
pyspectral/tests/test_rayleigh.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Collaborator

@adybbroe adybbroe left a comment

Choose a reason for hiding this comment

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

Many thanks for your analysis and the proposed changes. LGTM

@djhoese djhoese merged commit 1cd41c9 into pytroll:main Oct 10, 2022
@djhoese djhoese deleted the bugfix-revert-nantonum-rayl branch October 10, 2022 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants