Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exp test patterns #15

Closed
wants to merge 13 commits into from
Closed

Conversation

pzirali
Copy link
Collaborator

@pzirali pzirali commented Feb 26, 2024

Conditional Compliance for Test Patterns

@sandypreiss sandypreiss changed the base branch from main to experiment March 7, 2024 13:28
@sandypreiss sandypreiss changed the base branch from experiment to exp-crccp-sensitivity_01 March 7, 2024 13:29
Copy link
Collaborator

@sandypreiss sandypreiss left a comment

Choose a reason for hiding this comment

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

I think you're on the right track with the scenarios you've created, but the transform functions are not adjusting the right parameters. See my inline comment on those functions.

Please also convert this to a Draft PR so we don't accidentally merge it. All experiment PRs should be Draft PRs. We just use the PR interface for code review; we don't ever end up merging the experiment branches.

crcsim/experiment/prepare.py Outdated Show resolved Hide resolved
crcsim/experiment/prepare.py Outdated Show resolved Hide resolved
crcsim/experiment/prepare.py Outdated Show resolved Hide resolved
crcsim/experiment/prepare.py Outdated Show resolved Hide resolved
@sandypreiss
Copy link
Collaborator

@pzirali I corrected the transform functions. See the TODO comments for some places where you can fill in additional scenarios following the patterns of the ones I created.

The delayed onset adjustment ended up being more complicated than I initially thought. Make sure to take a close look at that transform function and check my logic.

Also, after you removed the sensitivity test scenarios, we still had the FQHC-specific baseline and implementation scenarios. Because we're messing with compliance rates in these scenarios, the FQHC-specific compliance rates won't be used, so we don't need separate scenarios for each FQHC in this experiment, just one scenario per testing pattern.

@sandypreiss
Copy link
Collaborator

This looks good! Can you confirm that the experiment ran successfully? Once it has, we can go ahead and close this PR.

@pzirali
Copy link
Collaborator Author

pzirali commented Mar 14, 2024 via email

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