-
Notifications
You must be signed in to change notification settings - Fork 80
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
(non) probabilistic E > I transition (SEIR) #157
Comments
of course, if you want to keep it similar to the one of the paper, also this should be changed as ~ |
Hi, If you like to address the discrepancy feel free to make a pull request, I'll be happy to merge it in the master branch! Giulio |
sorry, the implementation in this commit/line is wrong, as we should of course consider the time, since exposure |
closed due to #164 |
@GiulioRossetti @ggrrll Sorry to revisit this, but according to the paper referenced at the top of the thread, both E->I and I->R transition probabilities follow exponential distributions. Here's what it says in section 2:
Accordingly, I modified the code to reflect the same logic in transitioning from I to R. Didn't want to open a new issue, so I'm posting the suggestion below:
Apologize if this is the wrong way to go about. I'm a noob at using Github and didn't want to break anything by accident! |
@mihaibanciu sorry for the late reply Actually, this is something I have implemented few moths ago in two v. and we actually quickly chatted with @GiulioRossetti about it, some time ago... Indeed I was wondering whether we should have an 'exponential' implementation (like in branch: cont_time:SEIRModel ) or rather have all transitions in rate-like implementation (like in branch: SEIRModel:disc_time ) |
I think it should be done. The Exponential looks so different than the Uniform distribution and the models are so sensitive to initial parameters so that the final counts per each state could look very different if this is not addressed. BTW, in your branch you mention an error on L96 in the seir model. Is that a KeyError? I'm seeing the same. I wonder if one needs to initialize |
Sorry for the late reply. Indeed, it make sense to have both implementations in NDlib since we got several requests (both here and via email) asking for them. I think that, at this point, the optimal solution will be to add continuous versions for both SEIR and SEIS models. If you agree I'll give a look to your implementations to understand the nature of the error and move forward with the inclusion. |
@mihaibanciu @ggrrll check out the implementations in the dedicated branch. If they are fine, I'll push them into master and include the in the next release (that I would like to release within this week). @ggrrll: The error you faced was due to a missing initialization of infection seeds progress variable, as correctly pointed out by @mihaibanciu. To make things more readable I splitted the progress tracking in two disjoint dict. |
@GiulioRossetti As far as I can tell the |
Ok that's something that can be easily fixed :-) |
hi,
if I understood correctly, here
ndlib/ndlib/models/epidemics/SEIRModel.py
Line 90 in 450216a
you have a 'sharp' transition (step function, w.r.t time from exposure)
I think a probabilistic version would be better (more realistic), like the one I am attempting here:
ggrrll@6fcec70#diff-55a281f20b0ed52ec6ac2dc08e33ce64R91
that is basically the one mentioned in the paper you referenced
The text was updated successfully, but these errors were encountered: