Skip to content

Refine AscertainmentModel component to allow for time-varying component - #897

Open
cdc-mitzimorris wants to merge 75 commits into
mainfrom
mem_890_time_varying_ascertainment
Open

cdc-mitzimorris wants to merge 75 commits into
mainfrom
mem_890_time_varying_ascertainment

Conversation

@cdc-mitzimorris

Copy link
Copy Markdown
Collaborator

AscertainmentModel now supports optional signal-specific temporal processes, combining scalar baselines with logit-scale deviations while centralizing validation, sampling, and deterministic outputs. The branch adds IndependentAscertainment, refactors joint and ratio-linked models around baseline sampling, and updates model/observation plumbing to apply scalar or full-axis ascertainment rates after delay convolution.

  • Refactors count observations to apply ascertainment after delay convolution, supporting scalar or time-varying rates.
  • Updates MultiSignalModel/builder integration, including calendar-aligned temporal processes.
  • Revises tutorials and adds extensive unit/integration tests.

Suggested review order:

  1. pyrenew/ascertainment/base.py — understand the new baseline/temporal-process API and validation.
  2. pyrenew/ascertainment/independent.py, joint.py, and linked.py — review the concrete implementations.
  3. pyrenew/model/multisignal_model.py and pyrenew_builder.py — trace sampling and model integration.
  4. pyrenew/observation/count_observations.py and convolve.py — verify how time-varying rates are applied after delay convolution.
  5. Review test/test_ascertainment.py, then observation/builder tests, and finish with the tutorial changes.

@github-actions

github-actions Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Thank you for your contribution @cdc-mitzimorris 🚀! Your github-pages is ready for download 👉 here 👈!
(The artifact expires on 2026-10-14T19:16:50Z. You can re-generate it by re-running the workflow here.)

Copilot AI 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.

Copilot review overview

🟡 Changes recommended

The ascertainment tutorial references a nonexistent class and contains incorrect mathematical notation.

Review effort: Balanced
Findings: 3 Low severity

Open (3)
What changed in this PR

Adds scalar and time-varying ascertainment processes, integrates them across models and observations, and expands documentation and tests.

Changes:

  • Adds independent and temporal ascertainment APIs.
  • Applies ascertainment after delay convolution.
  • Adds extensive validation and integration coverage.
File Description
pyrenew/​ascertainment/​base.py Centralizes sampling and validation.
pyrenew/​ascertainment/​independent.py Adds independent ascertainment.
pyrenew/​ascertainment/​joint.py Supports temporal deviations.
pyrenew/​ascertainment/​linked.py Supports temporal deviations.
pyrenew/​ascertainment/​__init__.py Exports the new class.
pyrenew/​convolve.py Adds delay-only convolution.
pyrenew/​observation/​base.py Uses delay-only convolution.
pyrenew/​observation/​count_observations.py Applies ascertainment after delay.
pyrenew/​model/​multisignal_model.py Samples full-axis ascertainment.
pyrenew/​model/​pyrenew_builder.py Documents updated integration.
test/​test_ascertainment.py Tests the new API.
test/​test_convolve.py Tests delay-only convolution.
test/​test_helpers.py Updates convolution helper usage.
test/​test_observation_counts.py Tests time-varying rates.
test/​test_pyrenew_builder.py Tests model integration.
test/​integration/​conftest.py Configures independent ascertainment.
test/​integration/​test_population_infections_he_weekly_joint_ascertainment.py Verifies joint baseline sites.
test/​integration/​test_population_infections_he_weekly_ratio_linked_ascertainment.py Verifies linked baseline sites.
docs/​tutorials/​ascertainment.qmd Documents ascertainment models.
docs/​tutorials/​building_multisignal_models.qmd Updates convolution example.
docs/​tutorials/​observation_processes_counts.qmd Explains revised ordering.
docs/​tutorials/​observation_processes_measurements.qmd Updates convolution example.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread docs/tutorials/ascertainment.qmd Outdated
Comment thread docs/tutorials/ascertainment.qmd Outdated
Comment thread docs/tutorials/ascertainment.qmd Outdated
@codecov

codecov Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.35897% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 98.85%. Comparing base (673942b) to head (836e381).
⚠️ Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
pyrenew/observation/count_observations.py 94.44% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #897      +/-   ##
==========================================
+ Coverage   98.82%   98.85%   +0.02%     
==========================================
  Files          58       59       +1     
  Lines        2128     2264     +136     
==========================================
+ Hits         2103     2238     +135     
- Misses         25       26       +1     
Flag Coverage Δ
unittests 98.85% <99.35%> (+0.02%) ⬆️

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

☔ View full report in Codecov by Harness.
📢 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.

Comment thread pyrenew/ascertainment/base.py Outdated
@cdc-mitzimorris

Copy link
Copy Markdown
Collaborator Author

For a stronger end-to-end test than the integration test in this PR, I ran 4 PyRenew models using the time-varying IndividualAscertainment component on the NSSP (ED visit) signal on a grid of 2262 date/location/disease combinations for a 21-day forecast. the resulting CRPS scores were better than that of pyrenew-hew's E model on the same data.
The model is sound, but the implementation details still warrant human-review.

Comment thread docs/tutorials/ascertainment.qmd Outdated
Comment thread docs/tutorials/ascertainment.qmd
Comment thread docs/tutorials/ascertainment.qmd Outdated
Comment thread docs/tutorials/ascertainment.qmd Outdated
Comment thread docs/tutorials/ascertainment.qmd Outdated
Comment thread docs/tutorials/ascertainment.qmd Outdated
Comment thread docs/tutorials/ascertainment.qmd Outdated
Comment thread docs/tutorials/ascertainment.qmd Outdated
Comment thread docs/tutorials/ascertainment.qmd Outdated

@dylanhmorris dylanhmorris left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks @cdc-mitzimorris. Reviewed the tutorial carefully and made some suggestions. Also think we should have a code examples for:

  • IndependentAscertainment (either/both fixed in time or time varying)
  • A case of time-varying dependent ascertainment

Comment thread docs/tutorials/observation_processes_counts.qmd
Comment thread pyrenew/ascertainment/base.py
Comment thread pyrenew/ascertainment/base.py Outdated
Comment thread pyrenew/convolve.py

@dylanhmorris dylanhmorris left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Many thanks @cdc-mitzimorris! Very excited for this. Had a few comments/questions/suggestions.

@cdc-mitzimorris

Copy link
Copy Markdown
Collaborator Author

@dylanhmorris - ready for re-review.

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.

4 participants