Skip to content

Expose n_resamples on EvalBase to make bootstrap CI cost configurable#556

Merged
smcolby merged 2 commits into
mainfrom
fix/555/expose-n-resamples
May 21, 2026
Merged

Expose n_resamples on EvalBase to make bootstrap CI cost configurable#556
smcolby merged 2 commits into
mainfrom
fix/555/expose-n-resamples

Conversation

@smcolby
Copy link
Copy Markdown
Contributor

@smcolby smcolby commented May 20, 2026

Description

EvalBase.stat_and_bootstrap called scipy.stats.bootstrap with a hardcoded default of n_resamples=9999, causing ~60,000 bootstrap iterations on toy 4-element test data across 6 classification metrics. This results in test_classification_metrics taking ~18s with no way to reduce it from outside the class.

Closes #555.

Changes

openadmet/models/eval/eval_base.py

  • Added Field to pydantic imports
  • Added n_resamples: int = Field(default=9999, ge=1) to EvalBase; default preserves existing production behavior
  • Passed n_resamples=self.n_resamples to both bootstrap() calls in stat_and_bootstrap

openadmet/models/tests/unit/eval/test_eval.py

  • Instantiate ClassificationMetrics(n_resamples=100) and RegressionMetrics(n_resamples=100) in metric tests (CI precision is not under test there)

Performance

Test Before After
test_classification_metrics ~18.6s ~0.18s
test_regression_metrics ~4.4s ~0.04s

Quality Assurance & AI Policy

To maintain project quality and respect maintainer bandwidth, please confirm the following:

  • Manual Verification: I have manually reviewed and tested the code in this PR.
  • AI-Assisted Content: If AI tools were used (e.g., Copilot, ChatGPT), I have personally verified the logic, edge cases, and compliance with the existing codebase. I confirm the code is not a "blind" AI generation.
  • Minimal Review: I believe this PR is in a state that requires minimal intervention or correction from maintainers.
  • Scoped Change: This PR addresses a single, well-scoped issue rather than multiple unrelated changes.

Status

  • Ready to go (Checking this signals to maintainers that the PR is ready for final review)

Developers Certificate of Origin


Note to Contributors: We reserve the right to close PRs without review if they appear to lack human validation or do not meet the quality standards described in our CONTRIBUTING.md.

smcolby and others added 2 commits May 20, 2026 14:14
Add n_resamples: int = Field(default=9999) to EvalBase and thread it
into both bootstrap() calls in stat_and_bootstrap. Default preserves
existing production behaviour (scipy default is 9999).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ClassificationMetrics and RegressionMetrics are instantiated with
n_resamples=100 in tests that only assert metric values, not CI
precision. Cuts test_classification_metrics from ~18s to ~0.2s and
test_regression_metrics from ~4s to ~0.04s.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@smcolby smcolby self-assigned this May 20, 2026
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 20, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@smcolby smcolby marked this pull request as ready for review May 21, 2026 00:46
@smcolby smcolby merged commit 173f4e5 into main May 21, 2026
5 checks passed
@smcolby smcolby deleted the fix/555/expose-n-resamples branch May 21, 2026 00:47
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] Expose n_resamples in call to scipy.stats.bootstrap

3 participants