Skip to content

Conversation

@JoerivanEngelen
Copy link
Contributor

Fixes #1683

Description

This PR salvages the logic from #1686 to dump and import files from zarr and zip store and adds unit tests.

Checklist

  • Links to correct issue
  • Update changelog, if changes affect users
  • PR title starts with Issue #nr, e.g. Issue #737
  • Unit tests were added
  • If feature added: Added/extended example
  • If feature added: Added feature to API documentation
  • If pixi.lock was changed: Ran pixi run generate-sbom and committed changes

Manangka and others added 30 commits October 13, 2025 19:13
…al applicalble and some unittests are failing
… the way the solution is updated in the split model
…idn't expect to recieve dask objects. Optimize the flow-transport model matcher

(cherry picked from commit 9c5134a)
Base automatically changed from issue_#1698_model_write_optimization to master October 30, 2025 15:34
@Manangka Manangka changed the base branch from master to issue_#1698_model_splitter_optimization October 30, 2025 16:23
@Manangka Manangka changed the base branch from issue_#1698_model_splitter_optimization to master October 30, 2025 16:23
@Manangka
Copy link
Collaborator

serializer.zip

@JoerivanEngelen I've add a patch with a view changes this PR can benefit from.
Instead of having all the logic of writing files spread over the simulation, model and packages classes I propose consolidating them in 1 single file.
We would end up with 2 files, 1 for the netcdf serialization and 1 for the zarr serialition. This makes the code also easy extendable to support serializing to other formats

@Manangka
Copy link
Collaborator

Manangka commented Oct 31, 2025

When running the unittests i get the following error often:

C:\dev\imod-python\.pixi\envs\default\Lib\site-packages\_pytest\unraisableexception.py:33: RuntimeWarning: deallocating CachingFileManager(<class 'netCDF4._netCDF4.Dataset'>, 'C:\\Users\\user\\AppData\\Local\\Temp\\pytest-of-user\\pytest-1287\\test_split_dump_netcdf4_0\\split\\tpt_d_0_tpt_d_1.gwtgwt.nc', mode='r', kwargs={'clobber': True, 'diskless': False, 'persist': False, 'format': 'NETCDF4'}, manager_id='bf20bf87-26fd-48ea-a1c7-17925b05b8f5'), but file is not already closed. This may indicate a bug.

I'm not sure if this has anything to do with your code or that it was already there. Could you verify if this is new or not and create a separate story to fix this if it is exsiting behaviour?

You can reproduce it using:
pixi run pytest -k test_split_dump[netcdf4]

@Manangka
Copy link
Collaborator

When running the zarr.zip tests i get warnings about duplicate names:

...

imod/tests/test_mf6/test_multimodel/test_mf6_modelsplitter_transport.py::test_split_dump[zarr.zip]
  C:\dev\imod-python\.pixi\envs\default\Lib\zipfile\__init__.py:1624: UserWarning: Duplicate name: 'ihc/zarr.json'
    return self._open_to_write(zinfo, force_zip64=force_zip64)

imod/tests/test_mf6/test_multimodel/test_mf6_modelsplitter_transport.py::test_split_dump[zarr.zip]
  C:\dev\imod-python\.pixi\envs\default\Lib\zipfile\__init__.py:1624: UserWarning: Duplicate name: 'cdist/zarr.json'
    return self._open_to_write(zinfo, force_zip64=force_zip64)

imod/tests/test_mf6/test_multimodel/test_mf6_modelsplitter_transport.py::test_split_dump[zarr.zip]
  C:\dev\imod-python\.pixi\envs\default\Lib\zipfile\__init__.py:1624: UserWarning: Duplicate name: 'model_name_1/zarr.json'
    return self._open_to_write(zinfo, force_zip64=force_zip64)

...

You can reproduce it using:
pixi run pytest -k test_split_dump[zarr.zip]

@JoerivanEngelen
Copy link
Contributor Author

When running the unittests i get the following error often:

C:\dev\imod-python\.pixi\envs\default\Lib\site-packages\_pytest\unraisableexception.py:33: RuntimeWarning: deallocating CachingFileManager(<class 'netCDF4._netCDF4.Dataset'>, 'C:\\Users\\user\\AppData\\Local\\Temp\\pytest-of-user\\pytest-1287\\test_split_dump_netcdf4_0\\split\\tpt_d_0_tpt_d_1.gwtgwt.nc', mode='r', kwargs={'clobber': True, 'diskless': False, 'persist': False, 'format': 'NETCDF4'}, manager_id='bf20bf87-26fd-48ea-a1c7-17925b05b8f5'), but file is not already closed. This may indicate a bug.

I'm not sure if this has anything to do with your code or that it was already there. Could you verify if this is new or not and create a separate story to fix this if it is exsiting behaviour?

You can reproduce it using: pixi run pytest -k test_split_dump[netcdf4]

This was already present in the unittests for a few months. I don't see it happening in actual use cases, so it might be something with our test bench or something. Might be that we need to explictly close files during test teardown or something.

@JoerivanEngelen
Copy link
Contributor Author

When running the zarr.zip tests i get warnings about duplicate names:

...

imod/tests/test_mf6/test_multimodel/test_mf6_modelsplitter_transport.py::test_split_dump[zarr.zip]
  C:\dev\imod-python\.pixi\envs\default\Lib\zipfile\__init__.py:1624: UserWarning: Duplicate name: 'ihc/zarr.json'
    return self._open_to_write(zinfo, force_zip64=force_zip64)

imod/tests/test_mf6/test_multimodel/test_mf6_modelsplitter_transport.py::test_split_dump[zarr.zip]
  C:\dev\imod-python\.pixi\envs\default\Lib\zipfile\__init__.py:1624: UserWarning: Duplicate name: 'cdist/zarr.json'
    return self._open_to_write(zinfo, force_zip64=force_zip64)

imod/tests/test_mf6/test_multimodel/test_mf6_modelsplitter_transport.py::test_split_dump[zarr.zip]
  C:\dev\imod-python\.pixi\envs\default\Lib\zipfile\__init__.py:1624: UserWarning: Duplicate name: 'model_name_1/zarr.json'
    return self._open_to_write(zinfo, force_zip64=force_zip64)

...

You can reproduce it using: pixi run pytest -k test_split_dump[zarr.zip]

Also noticed this, it's actually doing this:

image

I didn't know this was possible!

Relevant issue: Suppressing ZipFile duplication warning · Issue #129 · zarr-developers/zarr-python

@sonarqubecloud
Copy link

@JoerivanEngelen JoerivanEngelen merged commit b28b807 into master Oct 31, 2025
5 of 7 checks passed
@JoerivanEngelen JoerivanEngelen deleted the issue_#1683_support_zarr branch October 31, 2025 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

to_file / dump / etc should support Zarr with ZipStore specifically

3 participants