Skip to content

Experiment: Test Combining#46

Closed
korourke5 wants to merge 8 commits intoexperiment-templatefrom
exp-test-combining
Closed

Experiment: Test Combining#46
korourke5 wants to merge 8 commits intoexperiment-templatefrom
exp-test-combining

Conversation

@korourke5
Copy link
Collaborator

CRCsim PR Template

Description

Created new branch for test combining experiment combining FIT and Blood tests in serial and parallel. Changes:

  • Parameters.py: ran combine_tests.py and added these new parameters here.
  • Prepare.py: added new parameters in to scenarios.append()

PR Type

  • Feature (intended to merge)
  • Experiment (not intended to merge)

Experiment review checklist

  • This experiment branch has the exp- prefix
  • This experiment uses the appropriate crcsim commit hash (typically, the latest commit in the main branch). This ensures that the experiment uses the latest version of the model. Note: used experiment-template
  • This experiment uses the correct baseline parameters.json file
    • Correct screening start and end ages for the experiment
    • Latest values of all calibrated parameters
    • All necessary tests and latest test parameters
    • Latest cost parameters
  • My prepare.py script applies incidence rate ratio (IRR) adjustment if appropriate for this experiment
  • I have run prepare.py and:
    • Confirmed that all scenarios necessary for this experiment were created
    • Confirmed that no extraneous scenarios are in the scenarios/ directory (eg, uncommitted local changes from a previous experiment)
    • Spot-checked a sample of scenario parameter files to ensure that my scenario creation logic works as expected
  • I have updated crcsim/experiment/README.md with detailed information about the experiment's goals, scenarios, and corresponding AWS objects
  • I am opening this PR as a Draft PR

Experiment review process

All experiments must follow each step of this review process.

  1. Contributor checks all items in experiment review checklist
  2. Contributor opens Draft PR and tags a collaborator to review
  3. Reviewer conducts a thorough review, including pulling experiment branch, running prepare.py, and spot-checking scenarios
  4. Contributor and reviewer address any issues identified during review
  5. Reviewer explicitly approves PR
  6. Contributor builds image and pushes to ECR, copies experiment files to S3, and runs experiment
  7. Contributor analyzes experiment (eg crcsim/experiment/summarize.py)
  8. Contributor pushes results summary, so we have documentation of detailed results (eg crcsim/experiment/summary/summarized.xlsx)
  9. Contributor updates experiment README with a detailed summary of results
  10. Contributor closes this PR with an informative comment (eg, brief summary of results)

Note that experiment PRs are never merged into main! The PR is closed, and the experiment is maintained as a separate branch. That's why we keep all experiment PRs as drafts.

@sandypreiss sandypreiss changed the base branch from main to experiment-template September 10, 2025 12:35
Comment on lines +143 to +159
scenarios.append(
Scenario(name=f"Blood_{scenario}", params=get_default_params())
.transform(transform_initial_compliance(screening_rate))
.transform(transform_lesion_risk_alpha(1.19))
)

scenarios.append(
Scenario(name=f"FIT_Blood_TestCombiningMethod.PARALLEL_{scenario}", params=get_default_params())
.transform(transform_initial_compliance(screening_rate))
.transform(transform_lesion_risk_alpha(1.19))
)

scenarios.append(
Scenario(name=f"FIT_Blood_TestCombiningMethod.SERIAL_{scenario}", params=get_default_params())
.transform(transform_initial_compliance(screening_rate))
.transform(transform_lesion_risk_alpha(1.19))
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

After we modify the test combining script to set the proportion for combined tests to 0, the base params will assign FIT to all agents. So these tests also need to use transform_routine_test_proportion to set the proportion for the relevant test to 1 and all others to 0 (same pattern as Colonoscopy).

@sandypreiss
Copy link
Collaborator

This looks good, Kate! Reviewing the combined test parameters highlighted a couple of changes that I want to make to the test combining script before we proceed.

  • The naming is awkward since it uses the full enum value. We should just use the string, eg "_parallel" instead of "TestCombiningMethod.PARALLEL".
  • We should have the proportion default to 0. As is, your parameters have a proportion of 1 for FIT and for both combined tests. Which obviously doesn't make sense. We should also add some parameter validation to check that all proportions sum to 1. I'll just add an issue for that for now, and fix the combining in the short term.

I will make these changes in the experiment-template branch, then we can merge into this branch and into main.

I also added one comment to the prepare script for you to work on.

Finally, we need to get your development environment set up to avoid the lint-and-format check failure. I will talk you through that during our next meeting.

@sandypreiss
Copy link
Collaborator

sandypreiss commented Sep 10, 2025

Oh, one last thing - you'll see I changed the base branch from main to experiment-template. That makes the diff easier to read. I'll also specify that in the PR template.

@sandypreiss
Copy link
Collaborator

@korourke5 I pushed my changes to test combining to experiment-template. You can merge that branch into yours, respond to my other comment, and rerun prepare.py. That should get you ready to run the experiment.

template' into exp-test-combining
the most recent versions of FIT_Blood_parallel and
FIT_Blood_serial from updated combine_tests.py
@korourke5
Copy link
Collaborator Author

@sandypreiss I have updated the branch with the changes you mentioned above.

@korourke5
Copy link
Collaborator Author

@sandypreiss I have updated with the latest run. I wasn't sure exactly which results we were looking for so I have not yet updated the README.

@sandypreiss
Copy link
Collaborator

Bottom line is that the results look as they ought to with the combined test.

FIT + blood in parallel leads to less CRC and more diagnostic tests (ie, positive screenings) than either test alone. FIT + blood in serial leads to more CRC and fewer diagnostic tests than either test alone.

You can copy that exactly for the readme. Also, please delete the ~ prefixed version of the summary sheet.

@korourke5
Copy link
Collaborator Author

@sandypreiss This PR should be all set for closing out.

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.

2 participants