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

Normalize uid argument to set_prognoses #387

Closed
daniel-klein opened this issue Mar 15, 2024 · 2 comments
Closed

Normalize uid argument to set_prognoses #387

daniel-klein opened this issue Mar 15, 2024 · 2 comments
Labels
quick A small/quick fix refactor Code needs refactoring

Comments

@daniel-klein
Copy link
Contributor

set_prognoses is either

def set_prognoses(self, sim, target_uids, source_uids=None)

or

def set_prognoses(self, sim, uids, source_uids=None)

Let's pick one and be consistent.

@daniel-klein daniel-klein added the enhancement New feature or request label Mar 15, 2024
@cliffckerr
Copy link
Contributor

cliffckerr commented Mar 15, 2024

For the specific question -- my vote is for uids.

However, I don't especially like the name set_prognoses -- I feel this is maybe a holdover from Covasim where we pre-computed the prognoses for the entire disease course, but we won't necessarily be doing that here. In particular, what I see the set_prognoses function doing is:

  • Appending to the infection log
  • Modifying state values (e.g. self.susceptible[uids] = False)
  • In some cases, then actually setting prognoses (e.g. self.ti_primary[uids] = sim.ti + rr(self.pars.dur_exposed.rvs(uids) / sim.dt)

I wonder if we should rename make_new_cases to choose_cases, and set_prognoses to make_cases or something like that?

I also don't love "cases". For infectious diseases, "case" sometimes means only a diagnosed infection. I propose that we use the term infection in Infection and its subclasses, and do not define this method for Disease, since it may or may not be applicable. For example, I could imagine an Obesity module wouldn't involve a make_new_cases method that would flip agents from one state to another, but would instead track continuous variables, and over a certain threshold agents would be defined as being a "case".

@cliffckerr cliffckerr added refactor Code needs refactoring and removed enhancement New feature or request labels Mar 15, 2024
@cliffckerr cliffckerr added this to the Usability milestone Mar 16, 2024
@cliffckerr cliffckerr added MVP quick A small/quick fix labels May 20, 2024
@cliffckerr cliffckerr mentioned this issue May 28, 2024
4 tasks
@cliffckerr
Copy link
Contributor

Closed by #527

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
quick A small/quick fix refactor Code needs refactoring
Projects
None yet
Development

No branches or pull requests

2 participants