satpy icon indicating copy to clipboard operation
satpy copied to clipboard

Correct the sun azimuth angle range within satpy.

Open simonrp84 opened this issue 3 years ago • 3 comments

At present, the azimuth angles returned for sun and satellite are on different ranges. satpy.modifiers.angles._get_sun_azimuth_ndarray returns values in the range -180. -> 180. degrees. satpy.modifiers.angles._get_sensor_angles_ndarray returns values in the range 0. -> 360. degrees.

For things like Rayleigh correction this is not a problem as the angles are normalised on lines 56 and 57 of atmosphere.py. However, it would be better to make this normalisation directly on the solar angles before they're passed to any other functions. My reasoning for this is:

  1. Users that call either of the azimuth angle calculation functions do not currently know that the angles are on different ranges - this is not documented. This is obviously unwanted and potentially problematic if they don't check the angles themselves before use.
  2. Although only the azimuth difference is used in satpy at the moment this may not always be the case (for more complex atmospheric correction, f.ex). We should therefore be prepared for future changes by ensuring azimuths are correct.

This PR applies a very simple fix (taken from atmosphere.py) to correct the solar azimuth range. Both azimuths will now be in the 0 -> 360 range as expected.

simonrp84 avatar Aug 05 '22 10:08 simonrp84

Codecov Report

Merging #2166 (6d777d1) into main (0c2b965) will increase coverage by 0.00%. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #2166   +/-   ##
=======================================
  Coverage   94.19%   94.20%           
=======================================
  Files         295      296    +1     
  Lines       45393    45410   +17     
=======================================
+ Hits        42760    42777   +17     
  Misses       2633     2633           
Flag Coverage Δ
behaviourtests 4.65% <0.00%> (-0.01%) :arrow_down:
unittests 94.85% <100.00%> (+<0.01%) :arrow_up:

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

Impacted Files Coverage Δ
satpy/modifiers/angles.py 99.15% <100.00%> (+<0.01%) :arrow_up:
satpy/tests/modifier_tests/test_angles.py 100.00% <100.00%> (ø)
satpy/tests/test_modifiers.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

codecov[bot] avatar Aug 05 '22 10:08 codecov[bot]

Coverage Status

Coverage increased (+0.002%) to 94.798% when pulling 6d777d1173d78f23d988fd866c77405a14bd0896 on simonrp84:fix_solar_angle into 0c2b9650e48867fbfb6292102400e3d5007bf74a on pytroll:main.

coveralls avatar Aug 05 '22 10:08 coveralls

@adybbroe or @mraspaud any memory of why the azimuth angle is in a different range in pyorbital?

@simonrp84 I'm tempted to request that the rayleigh correction be updated to only do the % 360 when data is loaded from provided "optional prerequisites" but avoids the extra calculation if the get_angles functions were used. Thoughts?

Also, tests please.

djhoese avatar Aug 05 '22 13:08 djhoese

I've added some tests to this now. What I did was rearrange the tests as was done in #2175, I moved a bunch of tests from test_modifiers.py into modifier_tests\test_angles.py. I then added a test_solazi_correction function to the test_angles.py file. Hopefully that enables either #2175 or this PR to be merged without causing issues when merging the other PR later.

simonrp84 avatar Nov 09 '22 11:11 simonrp84