Skip to content

Fix resampling when records exist preceding the simulation period - #1953

Merged
ClaireDons merged 4 commits into
masterfrom
issue_#1952-resample-timeseries
Nov 3, 2026
Merged

ClaireDons merged 4 commits into
masterfrom
issue_#1952-resample-timeseries

Conversation

@ClaireDons

@ClaireDons ClaireDons commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #1952

Description

Fixed bug where if a record exists before the simulation period, it isn't dropped and is taken into the period mean. This wasn't caught by the tests because there wasn't a test with varying rates, so the mean was the same in any case. I added a test to catch it now.

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

@sonarqubecloud

Copy link
Copy Markdown

@JoerivanEngelen JoerivanEngelen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm fine with this, as this stuff is now only used to mimic iMOD5. However, if I recall correctly, I made the conscious decision to forward fill rates before the first time step of the simulation period (as that is what I would expect). We could later add some settings to let users tweak behavior somewhat.

@ClaireDons
ClaireDons added this pull request to the merge queue Nov 3, 2026
Merged via the queue into master with commit 9a91093 Nov 3, 2026
8 checks passed
@ClaireDons
ClaireDons deleted the issue_#1952-resample-timeseries branch November 3, 2026 11:20
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.

[Bug] - resample_timeseries: output shifted by one period when well records exist before the first time

2 participants