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

Minimal counterfactualism #19

Closed
wants to merge 1 commit into from
Closed

Minimal counterfactualism #19

wants to merge 1 commit into from

Conversation

afmagee42
Copy link
Contributor

@afmagee42 afmagee42 commented Dec 13, 2024

This PR takes #16 to the extreme and adds in some speed findings from #13.

Compared to main as of 3ba4e72, it appears to allow many more simulations per second (NB: simulations run for 3 generations)

R0 simulations per second (this PR) simulations per second (main)
R0 = 0.5 16335.4925 8754.9636
R0 = 1.0 5600.6857 2346.7396
R0 = 2.0 1220.8826 390.8667

Copy link
Contributor

@swo swo left a comment

Choose a reason for hiding this comment

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

After working in #19, I was thinking to go even one step further, which is:

  1. When creating a new person, draw their disease history (timing of biological latent & infectious period)
  2. Immediately check for passive or active detection. Figure out the detection time, if any
  3. Create an "effective"(?) infectious period, and draw infection times from that

In other words, why not just scrap counterfactualism 100%?

Somehow this seems like fewer changes than #16. Did I overengineer something?

@@ -12,7 +13,6 @@ def __init__(self, params: dict[str, Any], seed: Optional[int] = None):

def run(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably now just rename run_infections -> run

@afmagee42
Copy link
Contributor Author

Somehow this seems like fewer changes than #16. Did I overengineer something?

This just removes the counterfactual. That looks like it shuffles code around, adds maximum outbreak sizes, restructures the simulation loop, and alters the widget.

It also helps that I didn't think to actually re-do any of the tests, so this is at least one modified file short of being usable.

(I will say I don't like the time truncation in #16. If you want to simulate until a maximum number of infections you need to truncate by time, not by person. If I'm 3 infections short of the maximum, the next three infections could come from anyone in this generation, I shouldn't just let the first person infect three more and stop.)

@afmagee42
Copy link
Contributor Author

After working in #19, I was thinking to go even one step further, which is:

  1. When creating a new person, draw their disease history (timing of biological latent & infectious period)
  2. Immediately check for passive or active detection. Figure out the detection time, if any
  3. Create an "effective"(?) infectious period, and draw infection times from that

In other words, why not just scrap counterfactualism 100%?

I thought about doing something more like that. At the time I decided

  • Something feels more spiritually correct about "outside" stuff happening outside generate_infection_history(). That is, passive detection is a thing a person does to themself. Active detection happens to them from somewhere else.
  • The efficiency benefits right now aren't all that big. If we hit a point where simulating just the number and times that a person might infect others becomes expensive, then doing as little of that as possible makes sense. But right now it's pretty cheap to do that, so drawing a full history and gradually lobbing bits off doesn't seem likely to cost much.
  • This feels marginally more modular than baking interventions in more deeply.

That being said, I'm not against redoing it that way as long as you're not worried about the modularity bit.

@swo
Copy link
Contributor

swo commented Dec 13, 2024

This just removes the counterfactual. That looks like it shuffles code around, adds maximum outbreak sizes, restructures the simulation loop, and alters the widget.

Got it, yes, #16 was more of an overhaul

I don't like the time truncation in #16

Fair, we shouldn't be processing infections in a queue, but rather dealing with them in the order in which they happened. I fixed that.

@swo
Copy link
Contributor

swo commented Dec 13, 2024

In other words, why not just scrap counterfactualism 100%?

I thought about doing something more like that

Have another look at #16, which is in a (I think) stable but not pretty state

The nice thing in that approach is that we visit each infection only once

@swo
Copy link
Contributor

swo commented Dec 16, 2024

Recommend closing in favor of #16

@afmagee42 afmagee42 closed this Dec 16, 2024
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