pytroll / satpy

Python package for earth-observing satellite data processing
http://satpy.readthedocs.org/en/latest/
GNU General Public License v3.0
1.04k stars 287 forks source link

Add support for NOAA-21 in MiRS limb correction #2711

Closed djhoese closed 6 months ago

djhoese commented 6 months ago

In the near future we hope to have limb correction for NOAA-21 from the algorithm developers. In the mean time we (the CSPP Polar2Grid team - @kathys) will use the NOAA-20 coefficients as if they were meant for NOAA-20. This PR fixes the MiRS reader so it can detect coefficient files for other NOAA satellites, not just NOAA-20 as it did previously.

This PR also includes a lot of refactoring and fixes for preserving 32-bit floats instead of accidentally upcasting to 64-bit floats.

Note to @kathys: I'm not sure we knew this but Sfc_type (surface type) is never treated as a mask variable from what I can tell in the unit tests here. Due to the way these files are structured (and that some satellite's files have less metadata than others) we use the global "missing_value" attribute which triggers the reader code to say "let's check for fills and replace with NaN". So this converts 16-bit integers to 32-bit floats. I could fix this, but it would risk breaking existing behavior and I'm not sure we have the time/funding/desire to go down that road so I'm leaving the behavior as is. Let me know if you think otherwise. In the end it probably doesn't change anything for us in Polar2Grid either way (for surface_type that is).

codecov[bot] commented 6 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (989d9f0) 95.39% compared to head (fb21a71) 95.39%. Report is 2 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #2711 +/- ## ======================================= Coverage 95.39% 95.39% ======================================= Files 371 371 Lines 52690 52686 -4 ======================================= - Hits 50263 50260 -3 + Misses 2427 2426 -1 ``` | [Flag](https://app.codecov.io/gh/pytroll/satpy/pull/2711/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pytroll) | Coverage Δ | | |---|---|---| | [behaviourtests](https://app.codecov.io/gh/pytroll/satpy/pull/2711/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pytroll) | `4.16% <0.00%> (+<0.01%)` | :arrow_up: | | [unittests](https://app.codecov.io/gh/pytroll/satpy/pull/2711/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pytroll) | `96.01% <100.00%> (+<0.01%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pytroll#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

kathys commented 6 months ago

@djhoese I read your comments about the missing data and 16 bit integers versus 32 bit floats, and I agree with you, that it is not worth the risk of breaking the existing behavior. I do not know of anyone who is using the Sfc_type parameter right now from these MiRS files.

coveralls commented 6 months ago

Pull Request Test Coverage Report for Build 7399569795


Totals Coverage Status
Change from base Build 7379871277: 0.002%
Covered Lines: 50386
Relevant Lines: 52509

💛 - Coveralls
djhoese commented 6 months ago

Note: If NOAA-21 data is provided to this reader as-is it will fail to process because it will try to load/retrieve coefficient files that are not configured in the reader's YAML. We're currently doing this in our own custom configuration in Polar2Grid by replicating the NOAA-20 files for NOAA-21.

djhoese commented 6 months ago

I need this PR to continue the work I'm doing that has a pretty tight deadline. I'm going to merge this now and if anyone does a post-merge review I can address issues then.