Skip to content

Check the terminal period of the transition path against a separate resource-constraint tolerance - #1225

Open
arihantlodha-cmd wants to merge 1 commit into
PSLmodels:masterfrom
arihantlodha-cmd:terminal-period-rc-check
Open

arihantlodha-cmd wants to merge 1 commit into
PSLmodels:masterfrom
arihantlodha-cmd:terminal-period-rc-check

Conversation

@arihantlodha-cmd

Copy link
Copy Markdown
Contributor

What this does

Closes #1210. RC_TPI was a single scalar compared against the resource-constraint error at every period with np.any, so the terminal period could not be exempted without waiving the check across the whole path. The terminal period of a truncated transition is not a true steady state, so its RC error is routinely larger than the interior periods, and the only workaround was to raise RC_TPI globally, which silently allows mid-path violations of the same size (for example OG-UK sets RC_TPI = 0.2 to route around the terminal spike, which then lets a 0.2 interior violation pass).

Changes

  • Add an RC_TPI_terminal parameter.
  • Check the interior periods (RC_error[:-1]) against RC_TPI and the terminal period (RC_error[-1]) against RC_TPI_terminal.
  • _rc_error_message now names whichever tolerance the worst error breaches.

Behavior and compatibility

RC_TPI_terminal defaults to RC_TPI (1e-4), so default behavior is unchanged. A user who needs to exempt the truncation boundary can now loosen RC_TPI_terminal alone and keep the tight interior check, which restores the interior diagnostic that a global loosening removes.

Testing

  • A full default-T baseline transition solves with the new check in place.
  • tests/test_parameters.py passes (16 passed).
  • Added test_rc_error_message_names_terminal_tolerance: the message names RC_TPI_terminal for a terminal-period spike and RC_TPI for an interior one. The existing test_rc_error_message still passes (the helper's third argument is optional).

…esource-constraint tolerance

Closes PSLmodels#1210. RC_TPI was a single scalar compared against the
resource-constraint error at every period with np.any, so the terminal
period could not be exempted without waiving the check across the whole
path. The terminal period of a truncated transition is not a true steady
state, so its RC error is routinely larger than the interior periods, and
the only workaround was to raise RC_TPI globally, which silently allows
mid-path violations of the same size.

This adds an RC_TPI_terminal parameter and checks the interior periods
(RC_error[:-1]) against RC_TPI and the terminal period (RC_error[-1])
against RC_TPI_terminal. RC_TPI_terminal defaults to RC_TPI (1e-4), so
default behavior is unchanged; a user who needs to exempt the truncation
boundary can now loosen RC_TPI_terminal alone and keep the tight interior
check. The failure message names whichever tolerance the worst error
breaches.

Adds a test that the message names RC_TPI_terminal for a terminal-period
spike and RC_TPI for an interior one.
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 54.54545% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.16%. Comparing base (9baa4b2) to head (6ed370a).

Files with missing lines Patch % Lines
ogcore/TPI.py 54.54% 5 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1225      +/-   ##
==========================================
- Coverage   75.17%   75.16%   -0.02%     
==========================================
  Files          24       24              
  Lines        6031     6039       +8     
==========================================
+ Hits         4534     4539       +5     
- Misses       1497     1500       +3     
Flag Coverage Δ
unittests 75.16% <54.54%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
ogcore/TPI.py 52.98% <54.54%> (+0.13%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jdebacker

Copy link
Copy Markdown
Member

@arihantlodha-cmd What's the use case for doing this? Is this for some kind of test run? I won't want to make it easier for users to think they've found an equilibrium when they have not as the RC error often points to some model misspecification or a calibration issue.

This branch has not been deployed

No deployments
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.

Resource-constraint check cannot exempt the terminal period without waiving the whole path

3 participants