pytroll / satpy

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

Fix Rayleigh correction to use the same datatype as the input data #2954

Closed pnuu closed 1 month ago

pnuu commented 1 month ago

This is workaround for the Pyspectral Rayleigh correction that always uses float64 datatype.

This includes also few additional dtype checks for other modifiers and enhancements. Those changes are here because I originally started this to hunt a data upcasting in true color composite, but didn't end up finding the final issue and didn't want to discard all the changes.

I'll create an issue to Pyspectral to fix the data upcasting. Hopefully tomorrow. Or a direct fix if I see a simple solution.

codecov[bot] commented 1 month ago

Codecov Report

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

Project coverage is 96.09%. Comparing base (118fc93) to head (3f1076a). Report is 7 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #2954 +/- ## ======================================= Coverage 96.09% 96.09% ======================================= Files 377 377 Lines 55110 55125 +15 ======================================= + Hits 52960 52975 +15 Misses 2150 2150 ``` | [Flag](https://app.codecov.io/gh/pytroll/satpy/pull/2954/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/2954/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pytroll) | `3.94% <0.00%> (-0.01%)` | :arrow_down: | | [unittests](https://app.codecov.io/gh/pytroll/satpy/pull/2954/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pytroll) | `96.19% <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.

coveralls commented 1 month ago

Pull Request Test Coverage Report for Build 11517663375

Details


Totals Coverage Status
Change from base Build 11517264487: 0.003%
Covered Lines: 53219
Relevant Lines: 55318

💛 - Coveralls
djhoese commented 1 month ago

@pnuu so is the plan for this to be closed and not used? Theoretically pyspectral should have all the changes for dtype stuff, right? Also I really thought I got pyspectral working and retaining dtypes. Shoot.

pnuu commented 1 month ago

Well, maybe the tests I've added should stay. I'll have a look at how much needs to be done to Pyspectral so that it doesn't upcast stuff, and whether there are still something that is needed on Satpy. Like a kwarg or something.

pnuu commented 1 month ago

I'm converting this to a draft while I inspect the Pyspectral side of things.

pnuu commented 1 month ago

Right. There was nothing wrong in Pyspectral, it was just the clipping of input value of reduce_strength that produced int64 for reduce_strength=1 and thus promoted the float32 data to float64.

pnuu commented 1 month ago

So in short: this PR makes sure the inputs to Rayleigh correction has proper data type to keep the data from being upcast. And adds the unrelated checks to other tests.

mraspaud commented 1 month ago

@djhoese do you want to look at this? otherwise it's good to merge