Add disturbance timing and tests

[dalonsoa]

Description

First step towards the disturbance framework: the disturbance timing. Based on some inputs, a list of time indices when the disturbance model will run is created. This is then used at runtime to decide if the disturbance should run, given the current time index.

Fixes #1366

Type of change

• New feature (non-breaking change which adds functionality)
• Optimization (back-end change that speeds up the code)
• Bug fix (non-breaking change which fixes an issue)

Key checklist

• Make sure you've run the pre-commit checks: $ pre-commit run -a
• All tests pass: $ poetry run pytest

Further checks

• Code is commented, particularly in hard-to-understand areas
• Tests added that prove fix is effective or that feature works
• Relevant documentation reviewed and updated

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.86%. Comparing base (8a26398) to head (1d0e0be).
⚠️ Report is 2 commits behind head on develop.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #1372   +/-   ##
========================================
  Coverage    94.85%   94.86%           
========================================
  Files           71       71           
  Lines         7387     7399   +12     
========================================
+ Hits          7007     7019   +12     
  Misses         380      380           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[davidorme]

@dalonsoa This looks good and bang on spec.

So instead, I'm going to move the target as I have a few questions about my own proposed API and the user experience 😄

• Does it make sense to have defaults? I think the default of the disturbance happening every step is a surprising and unlikely use case. I don't think some other default is any more sensible. In retrospect, I think it is clearer to have no defaults - users must specify the disturbance timing?

• I think I've overcomplicated the constructor args.

  • The primary mode could be run_at: int | list[int] | None, which implicitly supports run_once.
  • If run_at is None then users can instead use the convenience shortcut run_every: tuple[int, int]. The tuple provides a configurable start point and step (2,5) -> [2, 7, ...]. We could allow a single int for only specifying the step and default to run first on step zero (5 -> [0, 5, ...]), but again that is a surprising default, so better to make it explicit?

I think those configuration options are easier to document and explain and give a simpler API without surprising behaviour.

[dalonsoa]

I agree. It would be best to enforce the user to provide the timing explicitly. That would be enforced when creating the input layer (see #1369).

About the input arguments, I agree with your plan. The first option (run_at) is clear. About the second one, I'd use exactly the same syntax as range, mainly because it is familiar to most people using python and widely documented. My only concern there is that the mandatory argument there is stop, and not start, so maybe we can switch to make start mandatory and stop optional? So it would be:

run_every = (`start`)  # Runs to the end, with step 1
run_every = (`start`, `step`)  # Runs to the end with step `step`
run_every = (`start`, `step`, `stop`)  # Runs to `stop` with step `step`

[davidorme]

@dalonsoa Your suggested run_every scheme sounds good to me. I suspect 95% of usage would by (start, step) but at some point someone is going to want to run some other repeating pattern.

[davidorme]

LGTM