Deltares / Deltares/imod-python

API inconsistency regarding layer coords and MetaSWAP ``.from_imod5_data`` methods

Open
#1,336 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
41
Forks
12
Avg merge
21h 8m
Merged PRs (30d)
1

Description

https://github.com/Deltares/imod-python/pull/1335 will implement an imod.msw.MetaSwapModel.from_imod5_data method. What is not tackled here is the following:

  1. imod.prj.open_projectfile_data returns grids for the CAP package with a layer coordinate
  2. This coordinate information can be potentially required for when coupling MetaSwapModels to other layers than layer 1 (when this is implemented)
  3. Right now, imod.msw.MetaSwapModel.from_imod5_data drops this information
  4. The package.from_imod5_data implementations require the layer coord to be dropped

This makes for inconsistent API: imod.msw.MetaSwapModel.from_imod5_data is OK with the layer coord that is always added by open_projectfile_data, whereas e.g. imod.msw.GridData.from_imod5_data requires it to be dropped.

This makes for an annoying inconsistency in the API, for when users try to manually import individual packages themselves. I don't see a big use-case for it, but it IS unexpected behaviour.

To get something which works is very easy: xarray has an errors: "ignore" argument for its xr.DataArray.isel method. My concern is more a matter of performance: I haven't tested yet what xarray does when it ignores errors: Does it return a copy or a view? If the first, this would mean that potentially a lot of data would be copied, as the same grids are used for different packages, therefore grids would be copied multiple times for each package. Some profiling on the LHM data would be good to do here to test whether this leads to big bottlenecks.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by comparing imod.prj.open_projectfile_data with the MetaSwapModel.from_imod5_data and package.from_imod5_data implementations, including imod.msw.GridData.from_imod5_data. Profile the LHM data path while testing xarray.DataArray.isel with errors="ignore". Done means the from_imod5_data APIs handle layer coordinates consistently without an unacceptable copying or performance cost.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend-api-design
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.