Skip to content

Issue #1772 Bug fix writing clipped idf_svat.#1781

Open
verkaik wants to merge 1 commit intomasterfrom
issue_#1772_indices_clipping_idf_svat
Open

Issue #1772 Bug fix writing clipped idf_svat.#1781
verkaik wants to merge 1 commit 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
@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 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(...) 

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!

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: No status

Development

Successfully merging this pull request may close these issues.

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

2 participants