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 GSICS calibration in SEVIRI HRIT reader. #1323

Merged
merged 2 commits into from
Aug 27, 2020
Merged

Fix GSICS calibration in SEVIRI HRIT reader. #1323

merged 2 commits into from
Aug 27, 2020

Conversation

simonrp84
Copy link
Member

@simonrp84 simonrp84 commented Aug 26, 2020

The SEVIRI readers (NAT + HRIT) have the option to use the default (IMPF) or updated (GSICS) calibration coefficients for transforming digital number into radiance.

The HRIT reader, however, has a bug that means the calibration is invalid if GSICS mode is selected.
This PR fixes the bug by multiplying the GSICS offset by the GSICS slope value, as is recommended in the user guide and as is already done in the NAT reader.

  • Tests passed
  • Passes flake8 satpy

@coveralls
Copy link

coveralls commented Aug 26, 2020

Coverage Status

Coverage increased (+0.004%) to 90.136% when pulling ac43217 on simonrp84:seviri_gsics_fix into 4aedf55 on pytroll:master.

@codecov
Copy link

codecov bot commented Aug 26, 2020

Codecov Report

Merging #1323 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1323   +/-   ##
=======================================
  Coverage   90.13%   90.13%           
=======================================
  Files         220      220           
  Lines       32510    32512    +2     
=======================================
+ Hits        29303    29305    +2     
  Misses       3207     3207           
Impacted Files Coverage Δ
satpy/readers/seviri_l1b_hrit.py 92.18% <100.00%> (ø)
satpy/tests/reader_tests/test_seviri_l1b_hrit.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 4aedf55...ac43217. Read the comment docs.

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.

LGTM, I trust you validated with real data?

@simonrp84
Copy link
Member Author

Yes, tested against 4 SEVIRI scenes. Plus, this PR uses the same method used in both the NAT reader and in the EUM ATBD - so we can be pretty confident it's now correct :)

@mraspaud
Copy link
Member

So if the same method is used in multiple places, we could refactor the code to use the same function, right? Not saying it should be done in this PR though...

@simonrp84
Copy link
Member Author

I think the readers are so different that it's not worth the effort, tbh.

@mraspaud
Copy link
Member

Ok, merging then.

@mraspaud mraspaud merged commit b0d8460 into pytroll:master Aug 27, 2020
@simonrp84 simonrp84 deleted the seviri_gsics_fix branch August 27, 2020 13:23
@sfinkens
Copy link
Member

sfinkens commented Sep 7, 2020

Oops, that was me not reading the documentation 🙈 Thanks for fixing this @simonrp84 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants