Opening a dataset initialises two Scene objects with two respective load calls

[ameraner]

SIFT is currently creating two Scene objects, and calling load on both of them:

1. Scene load
2. Scene load

The Scene#1 is initialised after the files selection, the Scene#2 after the datasets selection. Both load calls happen (obviously) after the datasets selection. But it seems that only load#2 is the one that makes it into the layers, load#1 is only used to get metadata.

We should try to optimise this. E.g. Scene#1 could be passed further on to avoid a new init and a new load call?

In general the Scene init and load are not very time consuming for most instruments, but for FCI, given the high number of files and NetCDF complexity, this is taking considerable time, particularly when loading complex composites.

[RudiLauster]

Hello Andrea @ameraner ,

after investigations I learned that passing on the Scene object is actually implemented! How it works:
The scene is being created initially via create_scenes of the common module during OpenFileWizard.validateCurrentPage as you already noted:

sift/uwsift/util/common.py

Line 86 in bb7bfe8

scn = Scene(filenames=file_group, reader_kwargs=reader_kwargs)

But after that the scenes object is placed to the "scenes" item in the kwargs for the importer to be invoked later:

sift/uwsift/__main__.py

Lines 1138 to 1140 in bb7bfe8

	importer_kwargs = {
	"reader": reader,
	"scenes": scenes,

And prior to calling the SatpyImporter constructor via aImporter.fromproduct the associated scene object is taken from the "scenes" entry of the kwargs

sift/uwsift/workspace/importer.py

Line 280 in bb7bfe8

scn = kwargs["scenes"].get(tuple(paths), None)

and then passed on as "scene" object to the kwargs for the SatpyImporter constructor:

sift/uwsift/workspace/importer.py

Line 316 in bb7bfe8

kwargs["scene"] = scn

sift/uwsift/workspace/importer.py

Line 319 in bb7bfe8

return cls(paths, workspace_cwd=workspace_cwd, database_session=database_session, **kwargs) # type: ignore

And here no scene object is being constructed if "scene" is available in the kwargs but just taken from there

sift/uwsift/workspace/importer.py

Lines 460 to 464 in bb7bfe8

	self.scn = kwargs.get("scene")
	self.merge_target = kwargs.get("merge_target")
	if self.scn is None:
	reader_kwargs = get_reader_kwargs_dict([self.reader])
	self.scn = Scene(filenames={self.reader: self.filenames}, reader_kwargs=reader_kwargs)

Here is the catch:
If you look at from_product you can see that the scn object is being passed only under this condition:

if prod.info[Info.KIND] in (Kind.POINTS, Kind.LINES, Kind.VECTORS, Kind.IMAGE):
  ...

👉 and the prod.info[Info.KIND] is in case of the FCI is Kind.IMAGE

So if I just add this to the condition, the scene is no longer re-created and the loading procedure is definitely faster!

But if I look to the comment above..

sift/uwsift/workspace/importer.py

Lines 309 to 313 in bb7bfe8

	# Since only products of the kinds POINTS, LINES and VECTORS need
	# conversion (at this time), the mechanism is only applied for these.
	# For an unknown reason reusing the scene breaks for FCI data,
	#
	# this Importer data reading magic is too confused.

... I wonder if that is outdated? There was no "breaking of the scene". Well, looking at the last sentence I assume someone had got lost in the complexity 😁

👉 So I would just push this first (super small) fix now and you can take a look. And I wonder if this is it?!

Anyway, I'll look into the scene loading itself, if there is still "double loading".

PS: I also tested that successfully with SEVIRI GII Bufr as I just had an according dataset at hand.

[djhoese]

It looks like the commit that added some of that logic is here:

f638943

But gets lost in the transition from EUM GitLab -> SSEC GitHub for the other context.

I'm a little confused by the del kwargs["scene"] followed by reassigning the scene. Why not just assign the kwargs["scene"] which will replace the old value.

The one thing I'm wondering about for "breaking of the scene" is it may be talking about when there are or have been issues with the Satpy reader holding on to open files that are needed in the dask computations. In those cases if you delete or replace that first Scene, for example after resampling, this closes the reader and therefore the file objects. This results in an error when data is actually computed.

[RudiLauster]

Hello David @djhoese ,

thank you for looking into this!

I'm a little confused by the del kwargs["scene"] followed by reassigning the scene. Why not just assign the kwargs["scene"] which will replace the old value.

I am not sure if you have made a typo there or if you have fallen into the trap as I did initially when reading ... "scene" vs "scenes" 😁
In either case: a proof that caring for Clean Code is a good thing 😁

So, the "scenes" is a map that contains all Scene objects created whereas the key is the list of the selected files.
And "scene" is the actual Scene object under the given files selection.
I don't know where the need of having a list of files as dict key came from, during my investigations "scenes" always contained only one entry with the current file selection.

The one thing I'm wondering about for "breaking of the scene" is it may be talking about when there are or have been issues with the Satpy reader holding on to open files that are needed in the dask computations. In those cases if you delete or replace that first Scene, for example after resampling, this closes the reader and therefore the file objects. This results in an error when data is actually computed.

As I just write this I checked something and by happy accident I could reproduce the problem that seems to be described in the close by comment "For an unknown reason reusing the scene breaks for FCI data," and what I think you also describe here.

If you select several files that are not in sequence (using ctrl + right mouse) then you get this error:

2025-09-15 16:21:47 ERROR queue:run:L82 Background task exception: 
Traceback (most recent call last):
  File "C:\Projects\RESPASE_SIFT\sift\uwsift\queue.py", line 78, in run
    for status in task:
  File "C:\Projects\RESPASE_SIFT\sift\uwsift\__main__.py", line 338, in _bgnd_open_paths
    for progress in self.document.import_files(paths, **importer_kwargs):
  File "C:\Projects\RESPASE_SIFT\sift\uwsift\model\document.py", line 309, in import_files
    self.activate_product_uuid_as_new_dataset(uuid, insert_before=insert_before, **importer_kwargs)
  File "C:\Projects\RESPASE_SIFT\sift\uwsift\model\document.py", line 195, in activate_product_uuid_as_new_dataset
    self._workspace.import_product_content(uuid, **importer_kwargs)
  File "C:\Projects\RESPASE_SIFT\sift\uwsift\workspace\simple_workspace.py", line 280, in import_product_content
    for update in gen:
  File "C:\Projects\RESPASE_SIFT\sift\uwsift\workspace\importer.py", line 1168, in begin_import_products
    area_info = self._area_to_sift_attrs(dataset.attrs["area"])
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Projects\RESPASE_SIFT\sift\uwsift\workspace\importer.py", line 1062, in _area_to_sift_attrs
    raise NotImplementedError(
NotImplementedError: Only AreaDefinition datasets can be loaded at this time. If you're trying to read data that needs resampling, like LEO data, remember to select a resampling method in the Open File Wizard, and set 'geometry_definition: SwathDefinition' in your reader config.
2025-09-15 16:21:47 DEBUG queue:_did_complete_task:L190 completed task False, and_then we do this...
Traceback (most recent call last):
  File "C:\Projects\RESPASE_SIFT\sift\uwsift\queue.py", line 191, in _did_complete_task
    todo(succeeded)
  File "C:\Projects\RESPASE_SIFT\sift\uwsift\__main__.py", line 360, in _bgnd_open_paths_finish
    raise ValueError("background open did not succeed")
ValueError: background open did not succeed

So, I'll investigate further at this point and I would be happy if I get your opinion on this 👍🏼

@ameraner FYI

[djhoese]

I am not sure if you have made a typo there or if you have fallen into the trap as I did initially when reading ... "scene" vs "scenes" 😁

Yup, that's the problem. Thanks.

[RudiLauster]

I now know the reason for the exception. The function SatpyImporter._area_to_sift_attrs expects the area which will be converted to "sift attributes" to be of type AreaDefinition.
From AreaDefinition you can get the lower left, upper right and the pixel sizes (cell dimensions) rightaway. So that happens normally when you open interconnected files like so:

However, when you open files that are not interconnected like so:

then you receive an object of type StackedAreaDefinition in _area_to_sift_attrs, and that is rejected:

sift/uwsift/workspace/importer.py

Lines 1051 to 1054 in bb7bfe8

	def _area_to_sift_attrs(area):
	"""Area to uwsift keys"""
	if not isinstance(area, AreaDefinition):
	raise NotImplementedError(

And this happens as a consequence when you allow the Kind.IMAGE in order to have the "scene object reuse mechanism" like I drafted here:

https://github.com/ssec/sift/pull/465/files#diff-fcf4d135b50fd855e20036357f0a47cc05af9381be855fdce48925cb86191ce4

The StackedAreaDefinition contains a collection of AreaDefinition objects which is related to the "non interconnected file clusters". (see second screenshot above).
The problem is that each of the AreaDefinition objects has distinct values for area_extent, pixel_size_x and pixel_size_y and the function _area_to_sift_attrs needs "one value each".

But I saw that in importer.py we actually deal with AreaDefinition vs. StackedAreaDefinition. This happens in that function:

sift/uwsift/workspace/importer.py

Line 806 in bb7bfe8

def _get_area_extent(area: Union[AreaDefinition, StackedAreaDefinition]) -> List:

So, I just tried to reuse _get_area_extent to bring the area definitions together. But it is not that easy as the values provided from satpy.resample.get_area_def yield somehow shifted y-coordinates:

👉 Well, at this point I miss background information. Is resampling at this point a wrong operation?
If so, how to deal with the list of AreaDefinition objects to get those effective "sift attributes" in _area_to_sift_attrs? This is how _area_to_sift_attrs grabs the area information:

origin_x: area_extent[0],  # == lower_(left_)x
origin_y: area_extent[3],  # == upper_(right_)y
cell_width: pixel_size_x,
cell_height: -nominal_area.pixel_size_y

This is the area information of a block of 10 interconnected FCI L1 files, it is one AreaDefinition object and all is fine:

~> area type: <class 'pyresample.geometry.AreaDefinition'>
     proj_str: +ellps=WGS84 +h=35786400 +lon_0=0 +no_defs +proj=geos +type=crs +units=m +x_0=0 +y_0=0
     area_extent: (-5567999.998550753, -5567999.998550753, 5567999.998550748, -2783999.9992753775)
     pixel_size_x: 999.9999997397181
     pixel_size_y: 999.9999997397183

This is the area information of a block of the same FCI L1 files, but one (number 9) is missing what makes them not interconnected anymore and so we get a StackedAreaDefinition object. The distinct AreaDefinition objects look like this:

~> area type: <class 'pyresample.geometry.StackedAreaDefinition'>
defs[0]:
        proj_str: +ellps=WGS84 +h=35786400 +lon_0=0 +no_defs +proj=geos +type=crs +units=m +x_0=0 +y_0=0
        area_extent: (-5567999.998550753, -3062999.999202759, 5567999.998550748, -2783999.9992753775)
        pixel_size_x: 999.9999997397181
        pixel_size_y: 999.9999997397188
defs[1]:
        proj_str: +ellps=WGS84 +h=35786400 +lon_0=0 +no_defs +proj=geos +type=crs +units=m +x_0=0 +y_0=0
        area_extent: (-5567999.998550753, -5567999.998550753, 5567999.998550748, -3340999.999130401)
        pixel_size_x: 999.9999997397181
        pixel_size_y: 999.9999997397181

So after using the function _get_area_extent (which itself uses satpy.resample.get_area_def) I get this consolidated AreaDefinition which is totally different for area_extent[3]:

     proj_str: +ellps=WGS84 +h=35786400 +lon_0=0 +no_defs +proj=geos +type=crs +units=m +x_0=0 +y_0=0
     area_extent: (-5567999.998550739, -5567999.998550739, 5567999.998550762, 5567999.998550762)
     pixel_size_x: 999.9999997397181
     pixel_size_y: 999.9999997397181

I don't know if this just a phantom or a clue, but cry.py logs a warning:
C:\DevEnv\miniconda3\envs\sift\Lib\site-packages\pyproj\crs\crs.py:1295: UserWarning: You will likely lose important projection information when converting to a PROJ string from another format. See:

👉 So, in summary:
Do we just have an issue with resampling? Or is resampling just the wrong approach here. If so, what is be the correct one to deal with that collection of AreaDefinition objects (in StackedAreaDefinition)?
If this becomes too sophisticated, we can also have a simple solution to not re-use Scene objects in aImporter.from_product in case we have non-interconnected files. I assume that this is also a use case that may occur not too often?

Any ideas?

[ameraner]

Hi Rudi, thank you for your investigations so far!
An aspect that might help here: the scn.load calls have a kwarg pad_data: when True, missing chunks of a FCI scene are padded automatically, and satpy always returns a square array, filled with NaN when needed, with AreaDefinition with full extents (-5567999.998550739, -5567999.998550739, 5567999.998550762, 5567999.998550762) as returned by satpy.resample.get_area_def; when False, a StackedAreaDefinition is generated, which is a collection of all the single AreaDefinitions of each file, and the data arrays are simply stacked on top of each other, even if not contiguous.

So, with merge_with_existing=False, which is the default as defined here

sift/uwsift/etc/SIFT/config/auto_update.yaml

Line 9 in 64e16ed

merge_with_existing: False # shall be activated only with active auto_update

, and the load call here

sift/uwsift/workspace/importer.py

Line 1090 in 84d66cf

self.scn.load(dataset_ids, pad_data=not merge_with_existing, upper_right_corner="NE")

, you should not actually get a StackedAreaDefinition.. so I'm a little confused how you ended up there actually.

The merge_with_existing=True branch was created for the "auto-update" mode in SIFT, where single files are loaded as they arrive into a monitored folder, so we deactivate on purpose the padding in order to add only the single stripes into the accumulating disc as the files come (rather than merging a fully padded square array over and over).

Also note that the issue of the resampling scene replacing the original scene and causing NetCDF failures, that Dave mentions, is handled explicitly when we resample here:

sift/uwsift/workspace/importer.py

Lines 1094 to 1101 in 84d66cf

	# About the next strange line of code: keep a reference to the
	# original scene to work around an issue in the resampling
	# implementation for NetCDF data: otherwise the original data
	# would be garbage collected too early.
	self.scn_original = self.scn # noqa - Do not simply remove
	# deactivating reduce_data, see https://github.com/pytroll/satpy/issues/2476
	self.scn = self.scn.resample(resampler="native", reduce_data=False)

If the StackedAreaDefinition is the only problem, and it can be bound to the merge_with_existing key, then maybe a simple solution, at least as a first patch at this, is to not reuse Scene objects if merge_with_existing=True.

[RudiLauster]

Hi Andrea @ameraner ,

So I tried with pad_data==True and yes, with that it works as expected - no more StackedAreaDefinition !

But there are three things we need to check:

1. In scn.load of method begin_import_products the argument "merge_with_existing" is actually bound to the importer kwargs "merge_target" via self.merge_target.
   And this seems not to be bound to data_reading->merge_with_existing, I checked that in the constructor of the SatpyImporter
2. If I force pad_data to True explicitly, then it works as you describen, no StackedAreaDefinition is being created. But it looks like it is quite slow, what means that the initial performance gain is gone ...
3. The scn.load of method SatpyImporter._load_all_datasets actually puts pad_data to False explicitly (no evaluation of the "setting merge_with_existing") and that is also causing the StackedAreaDefinition to be created. Question is if that is intention or not?

So, guess we still need to 📞 😁