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

Enumerate states & reduce dead removal #294

Merged
merged 2 commits into from
Feb 22, 2024
Merged

Enumerate states & reduce dead removal #294

merged 2 commits into from
Feb 22, 2024

Conversation

cliffckerr
Copy link
Contributor

~25% performance increase by:

  • Using enumerate() in State (very small performance increase, <5%)
  • By default only removing dead once every 10 timesteps (the rest of the performance increase, >20%)

@robynstuart
Copy link
Contributor

if we implement this change, wouldn't we need to go through all the modules and make sure that we were always conditioning on people being alive before selecting them for e.g. pair formation, disease acquisition/progression, etc?

@RomeshA
Copy link
Contributor

RomeshA commented Feb 22, 2024

Neat! @robynstuart's issue is a good one although arguably, don't we already have to deal with that problem by allowing users to set remove_dead=False?

@cliffckerr
Copy link
Contributor Author

It is a good point ... the baseline test didn't change, so it seems like it's working? But in any case (which seems to be happening now anyway, at least for SIR), it would be nice if "being dead" (boolean flag) was decoupled from having to resize the array (computationally expensive).

@cliffckerr cliffckerr merged commit 1ddb491 into main Feb 22, 2024
2 checks passed
@cliffckerr cliffckerr deleted the enumerate-states branch February 22, 2024 18:42
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.

None yet

3 participants