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

Feature: add support for HRFI imagery in the FCI L1c reader #2287

Merged
merged 27 commits into from
Dec 1, 2022

Conversation

ameraner
Copy link
Member

@ameraner ameraner commented Nov 18, 2022

This PR adds support for high-resolution full-disc HRFI imagery in the FCI L1c reader.

It was decided to keep the same channel names as in the FDHSI case, ignoring the added _hr inside the HRFI file channel names. This has the advantage that HRFI and FDHSI files can be used interchangeably: when HRFI data is provided together with FDHSI files, Satpy will automatically select the finest resolution version of a channel (similarly e.g. to the MODIS L1b reader). If needed, a user can manually define the wished resolution by defining the resolution kwarg in the dataset load call (e.g. scn.load(['vis_06'], resolution=1000))

The PR also refactors the FCI L1c and GEOSegmentVariableYAMLReader tests, to improve the usage of fixtures, to have clearer class names and to be able to use parametrised tests for double FDHSI and HRFI checks.

@ameraner ameraner requested a review from sjoro November 18, 2022 10:19
@ameraner ameraner self-assigned this Nov 18, 2022
@ameraner ameraner added enhancement code enhancements, features, improvements component:readers PCW Pytroll Contributors' Week pre-launch For instruments not yet launched labels Nov 18, 2022
Copy link
Member

@gerritholl gerritholl 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 the good work. I left some inline comments.

I think we also need another area in areas.yaml, mtg_fci_fdss_500m. Or is this part of another PR?

@codecov
Copy link

codecov bot commented Nov 18, 2022

Codecov Report

Merging #2287 (d73bdde) into main (4962f2a) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #2287   +/-   ##
=======================================
  Coverage   94.35%   94.35%           
=======================================
  Files         310      310           
  Lines       46554    46575   +21     
=======================================
+ Hits        43926    43947   +21     
  Misses       2628     2628           
Flag Coverage Δ
behaviourtests 4.59% <3.45%> (+<0.01%) ⬆️
unittests 94.99% <100.00%> (+<0.01%) ⬆️

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

Impacted Files Coverage Δ
satpy/_compat.py 69.38% <100.00%> (+2.72%) ⬆️
satpy/readers/fci_l1c_nc.py 98.23% <100.00%> (+0.04%) ⬆️
satpy/readers/yaml_reader.py 97.49% <100.00%> (-0.01%) ⬇️
satpy/tests/reader_tests/test_fci_l1c_nc.py 100.00% <100.00%> (ø)
satpy/tests/test_yaml_reader.py 99.61% <100.00%> (+<0.01%) ⬆️

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

@coveralls
Copy link

coveralls commented Nov 18, 2022

Coverage Status

Coverage increased (+0.002%) to 94.948% when pulling d73bdde on ameraner:feature_add_support_for_hrfi into 4962f2a on pytroll:main.

@ameraner
Copy link
Member Author

@gerritholl thanks for your review, I also added the new AreaDefinition for the 500m grid to the areas.yaml already in this PR now

@ameraner
Copy link
Member Author

In the process of handling the different grid types coming from the FCI L1c reader depending on the file type, I also now optimised/refactored a bit the handling of the missing segments computation in the GEOVariableSegmentYAMLReader. In particular, the collection of the position of the available segments is now postponed to the get_dataset call, instead of being done early during the Scene initialisation. As this was the only part of code that was requiring information from the file during the Scene init, this opens the door now for a new optimisation PR that avoids the file opening alltogether in the FCIL1cNCFileHandler/NetCDF4FileHandler init, that would significantly reduce the time needed to initialise a Scene.

@ameraner
Copy link
Member Author

ameraner commented Dec 1, 2022

The result of

from satpy import Scene
from glob import glob

scn = Scene(filenames=glob(path_to_testdata + "*BODY*.nc"), reader='fci_l1c_nc')
scn.load(['ir_105'], upper_right_corner='NE')
scn.save_datasets(overlay={"coast_dir": '/coastlines_shapefiles'})

is:

image

note that:

  • the path_to_testdata folder contains both FHDI and HRFI data, so Satpy has automatically decided to load the ir_105 channel from the HRFI files, outputting an image with 11136x11136 px, as expected from the 1km grid (instead of the 2km grid in the FDHSI files).
  • the test data from the EUM ground processors contains a proxy SEVIRI image, with some high-res LEO data put on top, in order to excercise the navigation processors on data with realistic resolutions.
  • some chunks have deliberately been removed from the folder, in order to trigger and test the padding process.

From a longer geolocation test code, here is an ROI over the Canaries, plotted for both the FDHSI and HRFI case (with padding activated). Note how the geolocation is the same, with the HRFI image being at a better resolution:
testimg-_hc_fdhsi_missing_canaries_ir_105
testimg-hc_hrfi_missing_canaries_ir_105

@ameraner
Copy link
Member Author

ameraner commented Dec 1, 2022

From my side this is ready for a second round of reviews :) One test run is failing due to the mitiff issues discussed on Slack (not concerned by this PR).

@ameraner ameraner requested review from gerritholl and pnuu December 1, 2022 14:28
Copy link
Member

@pnuu pnuu left a comment

Choose a reason for hiding this comment

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

Just few comments/suggestions, otherwise LGTM.

Conflicts:
	satpy/readers/fci_l1c_nc.py
	satpy/tests/reader_tests/test_fci_l1c_nc.py
… a small explanation in the get_segment_position_info docstring
@mraspaud mraspaud merged commit b8883bc into pytroll:main Dec 1, 2022
@ameraner ameraner deleted the feature_add_support_for_hrfi branch December 2, 2022 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:readers enhancement code enhancements, features, improvements PCW Pytroll Contributors' Week pre-launch For instruments not yet launched
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Add support for HRFI imagery in the FCI L1c reader
5 participants