Refactor SZA and cos(SZA) generation to reduce duplicate computations

[djhoese]

This is another step towards better performance regarding #1902. I haven't figured everything out yet, but this PR does seem to help a little bit. There is a small amount of duplicate logic between satpy and pyorbital regarding the sun_zenith_angle function which was just np.rad2deg(np.arccos(cos_zen)). As pointed out in #1902, I noticed that cos(SZA) calculations for the sunz corrector modifier were not being shared with the rayleigh correction which also needs to calculate SZA. This PR refactors the relevant functions and wraps parts in map_blocks to allow dask to reuse calculations when possible.

• Closes #xxxx
• Tests added

[codecov]

Codecov Report

Merging #1910 (c3303e1) into main (1227328) will increase coverage by 0.03%.
The diff coverage is 97.91%.

@@            Coverage Diff             @@
##             main    #1910      +/-   ##
==========================================
+ Coverage   93.39%   93.43%   +0.03%     
==========================================
  Files         273      275       +2     
  Lines       40622    40742     +120     
==========================================
+ Hits        37940    38067     +127     
+ Misses       2682     2675       -7     

Flag	Coverage Δ
behaviourtests	4.83% <29.16%> (+<0.01%)	⬆️
unittests	93.97% <97.91%> (+<0.01%)	⬆️

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

Impacted Files	Coverage Δ
satpy/modifiers/atmosphere.py	44.59% <ø> (+1.17%)	⬆️
satpy/utils.py	26.77% <ø> (+0.98%)	⬆️
satpy/modifiers/angles.py	96.64% <97.36%> (-0.07%)	⬇️
satpy/modifiers/geometry.py	87.30% <100.00%> (-0.20%)	⬇️
satpy/resample.py	79.34% <100.00%> (-0.69%)	⬇️
satpy/readers/fci_l1c_nc.py	97.93% <0.00%> (-0.05%)	⬇️
satpy/tests/reader_tests/test_fci_l1c_nc.py	100.00% <0.00%> (ø)
satpy/tests/reader_tests/test_seadas_l2.py	98.43% <0.00%> (ø)
satpy/readers/seadas_l2.py	97.82% <0.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 1227328...c3303e1. Read the comment docs.

[djhoese]

I should note that this PR reduced the overall memory usage of the AHI and ABI true_color generation. For ABI is went from ~20GB to ~16GB. Note this was with my special test script which does not use a writer (no geotiff saving).

[coveralls]

Coverage decreased (-0.003%) to 93.902% when pulling c3303e1 on djhoese:refactor-coszen-mapblocks into 85656a8 on pytroll:main.

[mraspaud]

Thanks for this PR, with the refactoring. I see there is quite some comment code, maybe it should be removed. Also, it feels like most of the wrapping could be done as a decorator, an be done in pyorbital.

[djhoese]

"Why leave the commented code?"

Because I made this PR after 10 hours of working on this as a "hack" that then turned into an actual PR. I'll clean it up today.

[mraspaud]

Thanks

[djhoese]

Also, it feels like most of the wrapping could be done as a decorator, an be done in pyorbital.

@mraspaud You mean do map_blocks as a decorator? Does that work?

[djhoese]

Oh you mean make a decorator in pyorbital that handles dask inputs with map_blocks or if passed numpy arrays then calls it directly. Right?

[djhoese]

@mraspaud I think I've addressed all your concerns except for moving some of this dask support to pyorbital. Pyorbital doesn't currently have any dask or xarray support as far as I know.

[mraspaud]

Oh you mean make a decorator in pyorbital that handles dask inputs with map_blocks or if passed numpy arrays then calls it directly. Right?

yes

[djhoese]

Obviously that has to be done in a separate PR (on pyorbital) but would you be OK with this being merged as-is and then working on pyorbital later?

[mraspaud]

Obviously that has to be done in a separate PR (on pyorbital) but would you be OK with this being merged as-is and then working on pyorbital later?

That's ok, let's hope we have the time to fix pyorbital soon. Do we have an issue for that? otherwise it might be good to create one now, so that we don't forget this too easily

[pnuu]

LGTM. Just one clarification to docstring.

[djhoese]

@mraspaud An issue has been created: pytroll/pyorbital#88

[mraspaud]

LGTM