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

Fix geo interpolation for aapp data #1918

Merged
merged 5 commits into from Dec 14, 2021

Conversation

ninahakansson
Copy link
Contributor

@ninahakansson ninahakansson commented Dec 3, 2021

Switched to use the GeoInterpolator to interpolate the latitudes and longitudes for aapp data.
This gives correct results also near longitude 180 degrees but it is slower.
Added a test that show that interpolation of longitudes fails near longitude 180 degrees for aapp data
before the Interpolator was updated. Test data from file hrpt_metop03_20211129_0631_15888.l1b.

  • Closes #xxxx
  • Tests added

Before fix:

S_NWC_avhrr_ch_16_37_11_12_metopc_00000_20211129T0631000Z_20211129T0652599Z_npole thumbnail

This is actual data from hrpt_metop03_20211129_0631_15888.l1b
satpy/tests/reader_tests/test_aapp_l1b.py Outdated Show resolved Hide resolved
satpy/tests/reader_tests/test_aapp_l1b.py Outdated Show resolved Hide resolved
satpy/tests/reader_tests/test_aapp_l1b.py Outdated Show resolved Hide resolved
satpy/tests/reader_tests/test_aapp_l1b.py Outdated Show resolved Hide resolved
satpy/tests/reader_tests/test_aapp_l1b.py Outdated Show resolved Hide resolved
satpy/tests/reader_tests/test_aapp_l1b.py Outdated Show resolved Hide resolved
satpy/tests/reader_tests/test_aapp_l1b.py Outdated Show resolved Hide resolved
satpy/tests/reader_tests/test_aapp_l1b.py Outdated Show resolved Hide resolved
satpy/tests/reader_tests/test_aapp_l1b.py Outdated Show resolved Hide resolved
satpy/tests/reader_tests/test_aapp_l1b.py Outdated Show resolved Hide resolved
@ninahakansson ninahakansson changed the title Added test that fails for lat/lon data near lon = 180 Fix geo interpolation for aapp data Dec 10, 2021
@codecov
Copy link

codecov bot commented Dec 10, 2021

Codecov Report

Merging #1918 (ab4b750) into main (3de4ff6) will increase coverage by 0.11%.
The diff coverage is 94.28%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1918      +/-   ##
==========================================
+ Coverage   93.41%   93.52%   +0.11%     
==========================================
  Files         275      277       +2     
  Lines       40731    41218     +487     
==========================================
+ Hits        38047    38548     +501     
+ Misses       2684     2670      -14     
Flag Coverage Δ
behaviourtests 4.78% <0.00%> (-0.04%) ⬇️
unittests 94.05% <94.28%> (+0.07%) ⬆️

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

Impacted Files Coverage Δ
satpy/readers/aapp_l1b.py 89.24% <66.66%> (-0.58%) ⬇️
satpy/tests/reader_tests/test_aapp_l1b.py 100.00% <100.00%> (ø)
satpy/resample.py 79.34% <0.00%> (-0.69%) ⬇️
satpy/readers/seviri_l1b_native.py 85.39% <0.00%> (-0.25%) ⬇️
satpy/modifiers/geometry.py 87.30% <0.00%> (-0.20%) ⬇️
satpy/modifiers/angles.py 96.64% <0.00%> (-0.07%) ⬇️
satpy/readers/ahi_hsd.py 97.25% <0.00%> (-0.05%) ⬇️
satpy/readers/utils.py 91.79% <0.00%> (ø)
satpy/dataset/dataid.py 95.16% <0.00%> (ø)
satpy/writers/geotiff.py 93.22% <0.00%> (ø)
... and 16 more

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 3de4ff6...ab4b750. Read the comment docs.

@coveralls
Copy link

coveralls commented Dec 10, 2021

Coverage Status

Coverage increased (+0.08%) to 93.985% when pulling ab4b750 on ninahakansson:fix_latlon_interp_aapp into 3de4ff6 on pytroll:main.

@adybbroe
Copy link
Contributor

@ninahakansson Great work. Can you please describe what your PR does and why, in addition from adding more tests? The current description doesn't match the title
And, run isort locallly and/or install pre-commit to your local repo to fix the imports.

@adybbroe
Copy link
Contributor

adybbroe commented Dec 10, 2021

@ninahakansson In your local satpy fork do:
pre-commit install

To fix for isort now you can do:

pre-commit run isort --all-files or

pre-commit run isort --files satpy/tests/reader_tests/test_aapp_l1b.py

Copy link
Member

@mraspaud mraspaud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for reporting and fixing this! I think we can just drop the regular interpolator.

satpy/readers/aapp_l1b.py Show resolved Hide resolved
@ninahakansson ninahakansson marked this pull request as ready for review December 14, 2021 08:37
Copy link
Member

@mraspaud mraspaud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mraspaud mraspaud merged commit a2b1e82 into pytroll:main Dec 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants