Track and write measurement time for area

[gerritholl]

Track measurement times such that a representative measurement time can be estimated and written for an area.

To include valid time in file headers for supported writers, add reader_kwargs: {"track_time": True} to Scene.__init__ and
resample_coords=True to Scene.resample.

When storing as ninjogeotiff, time coordinates will be averaged and stored in the ninjo_ValidTimeID tag.

An example is included in the PR under doc/source/examples/valid_time.rst.

Those two PRs must be merged first:

• Refactor _resampled_scene() and _reduce_data() methods of the Scene class #3178
• Add a unit test for the CO2 corrector #3371

Next steps, either within this PR or within one or more later PRs:

• Add other readers
• Add other writers
• Add a way to include the valid time in the filename (deferred)
• Investigate reduce_data performance penalty (see this comment)

The latter is difficult to do without triggering a computation, so doing this efficiently should probably be done as postprocessing beyond the scope of this PR.

import os
from satpy import Scene
from glob import glob
from satpy.utils import debug_on; debug_on()
seviri_files = glob("/media/nas/x21308/scratch/SEVIRI/202103300900/H-000*")
sc = Scene(filenames={"seviri_l1b_hrit": seviri_files}, reader_kwargs={"track_time": True})
sc.load(["IR_108"])
ls = sc.resample("nqeuro3km", resample_coords=True, resampler="gradient_search")
ls.save_dataset(
        "IR_108",
        "{platform_name}-{sensor}-{name}-{area.area_id}-{start_time:%Y%m%d%H%M}-geotiff.tif",
        writer="ninjogeotiff",
        ChannelID="IR 10.8",
        DataType="GORN",
        PhysicValue="Temperature",
        PhysicUnit="K",
        SatelliteNameID="METEOSAT Europe-Atlantic",
        fill_value=0)

• Closes Write "valid time" to filename or headers #3161
• Tests added
• Fully documented
• Add your name to AUTHORS.md if not there already

[codecov]

Codecov Report

❌ Patch coverage is 96.83099% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.36%. Comparing base (fbfbe21) to head (06de789).
⚠️ Report is 12 commits behind head on main.

Files with missing lines	Patch %	Lines
satpy/scene.py	87.05%	11 Missing ⚠️
satpy/composites/core.py	89.79%	5 Missing ⚠️
satpy/writers/core/base.py	60.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3168      +/-   ##
==========================================
+ Coverage   96.32%   96.36%   +0.04%     
==========================================
  Files         466      469       +3     
  Lines       59096    59452     +356     
==========================================
+ Hits        56922    57293     +371     
+ Misses       2174     2159      -15     

Flag	Coverage Δ
behaviourtests	3.63% <6.69%> (+0.04%)	⬆️
unittests	96.45% <96.83%> (+0.04%)	⬆️

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

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[coveralls]

Pull Request Test Coverage Report for Build 16744116940

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

• For more information on this, see Tracking coverage changes with pull request builds.
• To avoid this issue with future PRs, see these Recommended CI Configurations.
• For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

• 190 of 201 (94.53%) changed or added relevant lines in 13 files are covered.
• No unchanged relevant lines lost coverage.
• Overall first build on feature-valid-time at 96.38%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
satpy/scene.py	57	68	83.82%

Totals
Change from base Build 16697789205:	96.4%
Covered Lines:	56147
Relevant Lines:	58256

---

💛 - Coveralls

[gerritholl]

To track "valid time" through resampling for geostationary satellite data, I'm trying to think of a good way to approach this. My current thinking is that if the user creates the scene with reader_kwargs={"track_time": True}, the file handler adds per-pixel time information to the variable attributes. By putting it inside ancillary variables, dataset_walker and co ensure it gets sliced and resampled along with the parent dataset, so that at the end, an approximate measurement time can be estimated for the resampled scene. But normally ancillary variables are actual variables available in the data file, they remain text attributes until after segments have been concatenated, and then the file handler is called again for every segment, to replace the text label by actual data in ancillary variables. Therefore, I'm not sure if ancillary variables is the right place to put it, or if time should be stored there at all.

[gerritholl]

Maybe it fits better in a coordinate variable, but not sure if it's valid to have those be not-unique.

[gerritholl]

Coordinates can probably be non-unique, but are currently dropped in resampling. In case of nearest neighbour resampling, this is due to

https://github.com/pytroll/pyresample/blob/61628530636b596150f3b0f45d0a9e78a2d80f2c/pyresample/kd_tree.py#L1094-L1095

[gerritholl]

Addressing the CodeScene complaint about Scene._resampled_datasets is too difficult for me. I've looked at it for too long and tried different things, but I do not manage to improve this, sorry.

https://github.com/gerritholl/satpy/blob/329dab5489adf9dfa0b170be3cd16acc52f1674d/satpy/scene.py#L864-L907

I am not able to resolve this as part of this PR.

[gerritholl]

EDIT: This comment is out of date. There is no longer a performance penalty.

Original comment:

---

This PR is strongly increasing the overhead of reduce_data when resampling, even if time tracking is not switched on.

Calling Scene.resample:

branch	reduce_data	time tracking	caching	duration
main	False	N/A	False	0.97
main	True	N/A	False	8
main	False	N/A	True	0.96
main	True	N/A	True	0.97
3168	False	False	False	0.96
3168	False	False	True	0.96
3168	False	True	False	1.11
3168	False	True	True	1.20
3168	True	False	False	42.7
3168	True	False	True	0.99
3168	True	True	False	83.4
3168	True	True	True	1.2

NB: This is just for calling resample — no computation. See also #2620.

I don't yet understand why the overhead for resample with reduce_data=True increases from 8 to 43 seconds even if no time tracking is actually used. Needs to be investigated more.

[gerritholl]

For ninjogeotiff, the NinJo team have agreed this should go in a tag called ninjo_ValidTimeID (not ValidDateID).

(In case a NinJo developer or someone with access to the internal NinJo JIRA reads this: details in NinJo JIRA-38947).

[gerritholl]

The worsening performance with _reduce_data when not using time tracking is due to #3178.

The even-more-worsening performance with time tracking is because reduce data is called twice as often.

[gerritholl]

Both problems are solved with f70d80f. The superfluous calls to _reduce_data are not a problem any more (little overhead), because on the second run it finds the source areas in the reductions dictionary. The overhead merely increases from 8.3 to 8.6 seconds (previously from 8.3 to 83.4 seconds).