Skip to content

Issue #1772 Bug fix writing clipped idf_svat.#1781

Merged
JoerivanEngelen merged 2 commits intomasterfrom
issue_#1772_indices_clipping_idf_svat
Mar 17, 2026
Merged

Issue #1772 Bug fix writing clipped idf_svat.#1781
JoerivanEngelen merged 2 commits intomasterfrom
issue_#1772_indices_clipping_idf_svat

Conversation

@verkaik
Copy link
Contributor

@verkaik verkaik commented Mar 11, 2026

Fixes #1772

Description

Bug fix for row/column indices when writing a clipped version of idf_svat.inp.

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

@verkaik verkaik requested a review from JoerivanEngelen March 11, 2026 15:31
@verkaik verkaik added the bug Something isn't working label Mar 11, 2026
Copy link
Contributor

@JoerivanEngelen JoerivanEngelen 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 adding the test and proposing a fix! I'm not happy yet with the location of the fix though. If you can move the logic to IdfMapping._render (I gave some suggestions), we utilize the strengths of the current architecture a lot better.

Collect to be written data in a DataFrame and call
``self.write_dataframe_fixed_width``
"""
if self.__class__.__name__ == "IdfMapping":
Copy link
Contributor

Choose a reason for hiding this comment

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

This works, but we don't want special logic for IdfMapping in the Package baseclass (unless there is no other way).

In this case, you can override the _render method in the IdfMapping class, in the imod/msw/idf_mapping.py file. In pseudo-code:

class IdfMapping:
...
    def _render(
        self,
        ...
    ) -> None:
        
        <all logic to compute rows, cols, x_grid, and y_grid>

        self.dataset["rows"] = rows
        self.dataset["columns"] = columns
        self.dataset["x_grid"] = x_grid
        self.dataset["y_grid"] = y_grid

        super()._render(...) 

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, good one! I just changed the code like you suggested.

assert_almost_equal(results["x_grid"], np.array([2.0, 2.0, 2.0, 2.0, 4.0]))


def test_idf_oc_write_simple_model_clip(fixed_format_parser):
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice!

@sonarqubecloud
Copy link

Copy link
Contributor

@JoerivanEngelen JoerivanEngelen 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 changing this!

@JoerivanEngelen JoerivanEngelen merged commit 09cf5eb into master Mar 17, 2026
7 of 8 checks passed
@JoerivanEngelen JoerivanEngelen deleted the issue_#1772_indices_clipping_idf_svat branch March 17, 2026 08:17
@github-project-automation github-project-automation bot moved this to ✅ Done in iMOD Suite Mar 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

[Bug] - Incorrect row/col indices when clipping idf_svat.inp (MetaSWAP)

2 participants