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 support for NOAA-21 in MiRS limb correction #2711

Merged
merged 5 commits into from
Jan 5, 2024

Conversation

djhoese
Copy link
Member

@djhoese djhoese commented Jan 3, 2024

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).

  • Closes #xxxx
  • Tests added
  • Fully documented
  • Add your name to AUTHORS.md if not there already

@djhoese djhoese added bug component:readers cleanup Code cleanup but otherwise no change in functionality labels Jan 3, 2024
@djhoese djhoese self-assigned this Jan 3, 2024
@djhoese djhoese requested a review from mraspaud as a code owner January 3, 2024 16:05
Copy link

codecov bot commented Jan 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

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
@@           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 Coverage Δ
behaviourtests 4.16% <0.00%> (+<0.01%) ⬆️
unittests 96.01% <100.00%> (+<0.01%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kathys
Copy link

kathys commented Jan 3, 2024

@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
Copy link

Pull Request Test Coverage Report for Build 7399569795

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.002%) to 95.957%

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

💛 - Coveralls

@djhoese
Copy link
Member Author

djhoese commented Jan 3, 2024

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
Copy link
Member Author

djhoese commented Jan 5, 2024

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.

@djhoese djhoese merged commit afb4049 into pytroll:main Jan 5, 2024
18 of 19 checks passed
@djhoese djhoese deleted the feat-mirs-noaa21-coeff-handling branch January 5, 2024 01:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug cleanup Code cleanup but otherwise no change in functionality component:readers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants