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

Fix MODIS readers applying add_offset incorrectly #2142

Merged
merged 3 commits into from Jul 11, 2022

Conversation

djhoese
Copy link
Member

@djhoese djhoese commented Jul 11, 2022

It was pointed out on the mailing list that when reading MODIS L2 (MOD06) cloud top temperature that the resulting values were nonsensical. This is due to us applying scale_factor/add_offset in the wrong way. We were doing:

data = byte_value * scale_factor + add_offset

but we should be doing:

data = (byte_value - add_offset) * scale_factor

because MODIS.

This PR fixes this issue and others. While adding checks to the tests to verify that what was happening was actually happening, I noticed that byte mask conversions were actually converting dask to numpy arrays. I've fixed this issue as well by wrapping these operations in a da.map_blocks call.

Note: This PR does not include the test fixes in #2141 so the tests will fail until that is merged.

  • Closes #xxxx
  • Tests added
  • Fully documented

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 goods, just two minor suggestions.

satpy/readers/hdfeos_base.py Show resolved Hide resolved
satpy/readers/modis_l2.py Outdated Show resolved Hide resolved
@simonrp84
Copy link
Member

Just tested this on MODIS L1 data, no effect on the results - which is what we expect as these use a 0 add_offset.

@codecov
Copy link

codecov bot commented Jul 11, 2022

Codecov Report

Merging #2142 (442cccd) into main (607c25b) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #2142   +/-   ##
=======================================
  Coverage   93.88%   93.88%           
=======================================
  Files         283      283           
  Lines       43106    43123   +17     
=======================================
+ Hits        40470    40488   +18     
+ Misses       2636     2635    -1     
Flag Coverage Δ
behaviourtests 4.78% <0.00%> (-0.01%) ⬇️
unittests 94.54% <100.00%> (+<0.01%) ⬆️

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

Impacted Files Coverage Δ
satpy/readers/hdfeos_base.py 92.27% <100.00%> (+0.40%) ⬆️
satpy/readers/modis_l2.py 89.81% <100.00%> (+0.29%) ⬆️
satpy/tests/reader_tests/_modis_fixtures.py 97.61% <100.00%> (+0.01%) ⬆️
satpy/tests/reader_tests/test_modis_l1b.py 100.00% <100.00%> (ø)
satpy/tests/reader_tests/test_modis_l2.py 100.00% <100.00%> (ø)

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 607c25b...442cccd. Read the comment docs.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.005%) to 94.484% when pulling 442cccd on djhoese:bugfix-modis-scale-offset into 607c25b on pytroll:main.

@mraspaud mraspaud merged commit 15dbf3d into pytroll:main Jul 11, 2022
@djhoese djhoese deleted the bugfix-modis-scale-offset branch July 11, 2022 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants