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

Add simpleabm #339

Merged
merged 9 commits into from
Mar 10, 2024
Merged

Add simpleabm #339

merged 9 commits into from
Mar 10, 2024

Conversation

cliffckerr
Copy link
Contributor

@cliffckerr cliffckerr commented Feb 29, 2024

Update: SimpleABM now lives at http://github.com/starsimhub/simpleabm. This PR instead provides a fix to StaticNet, so it works with no input arguments.

Implements a SimpleABM module, which is an as-simple-as-reasonable SIR (for now) ABM, including a spatial network.

Definitely not the PR I thought I'd be making now, but I ended up doing this for debugging Starsim's SIR model, and I feel like it might be useful for (a) debugging to know what the "simple" answer should be (eg do my SIR parameters make sense?), (b) rapid prototyping (eg a new network structure), (c) teaching (eg a rough approximation of what Starsim itself is doing).

Closes #124

@daniel-klein
Copy link
Contributor

I don’t think we want to include simpleabm within Starsim. Or the comparison for that matter. Consider as a separate repo?

@cliffckerr
Copy link
Contributor Author

I've gone back and forth on this, as illustrated by me opening, then closing, then reopening the original issue! But I've long thought something like this could be useful for testing and debugging (as it has been for SIR already, and maybe e.g. different networks down the track). If it would eventually be incorporated in CI tests, then it would be nice to have it in the same repo. But keen for everyone's input!

@kaa2102
Copy link
Contributor

kaa2102 commented Feb 29, 2024

I've found simpleabm to be useful for the purpose of comparison and debugging. Also, the implementation of the SIR calculations are different from the Starsim SIR calculations/methods I observed in tests/test_simple.py + test_sir_epi(). I agree with incorporating this into CI tests.

@daniel-klein
Copy link
Contributor

Including simpleabm as a separate module in the Starsim framework has potential to create significant confusion. It is not compatible with Starsim and gives different answers. I think it belongs as part of our training materials considering it lacks capabilities and performance as required for actual (re)use.

@cliffckerr
Copy link
Contributor Author

cliffckerr commented Feb 29, 2024

I think that's a good question -- does it give different answers? My feeling is that it should be possible for Starsim (specifically, ss.Sim(diseases='sir', networks='static')) to reproduce the results of SimpleABM, stochastic effects notwithstanding. If it can't, I think that's a bug!

I also have a different feeling about the second part -- a lot (perhaps too much) of "actual" work in epidemiology is done with simple SIR-derived models. I could see SimpleABM having genuine use as a starting point for a simpler and less black-box model. I say this because I was surprised how much participants at the Makerere training liked it (and used it as a starting point for their policy-relevant modeling questions), and I think we have an opportunity to embrace this impulse rather than try to redirect it towards a tool that might be overly complex for their needs.

The question remains whether this belongs in a separate repo, though. I could definitely see an argument that it does, since it's an almost entirely separate codebase (although I do think it would be cool if we could make network objects compatible with either). But I think Starsim and SimpleABM do have an opportunity to strengthen each other if paired -- e.g., for someone who is inclined to use SimpleABM, it lowers the barrier for entry to switch to Starsim.

I also think there's an opportunity to make SimpleABM a little less simple and more flexible/performant -- but that is probably not on the critical path here, unless we want to more formally support this for use in the upcoming training for less technically focused participants.

@daniel-klein
Copy link
Contributor

Agree it's useful, it just doesn't belong within the Starsim framework repo.

@RomeshA
Copy link
Contributor

RomeshA commented Mar 8, 2024

If it was in this repo, then putting it somewhere like the tests folder would make more sense, it is quite confusing for it to sit at the top alongside starsim. But I think a separate repo under starsimhub would be my preference too

@menriquez-IDM
Copy link
Contributor

Maybe having a starsim-sandbox or starsim-samples will be a good place to share implementations like this. I see a lot of value on keeping them around in a visible spot.

@cliffckerr cliffckerr mentioned this pull request Mar 9, 2024
@cliffckerr cliffckerr merged commit 5e36efb into main Mar 10, 2024
2 checks passed
@cliffckerr cliffckerr deleted the add-simpleabm branch March 10, 2024 03:17
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.

Build minimal ABM testbed
5 participants