1003 implement ODE SEIR metapopulation model#1273
1003 implement ODE SEIR metapopulation model#1273charlie0614 wants to merge 138 commits intomainfrom
Conversation
…s-integrated-into-the-odes
…s-integrated-into-the-odes
HenrZu
left a comment
There was a problem hiding this comment.
Great work :) Only some comments, and then we can merge :)
HenrZu
left a comment
There was a problem hiding this comment.
Sorry for the delay. Let's get this done soon. Overall, it looks very good and is an interesting addition. However, we still need to work on the presentation and documentation :)
cpp/examples/ode_seir_metapop.cpp
Outdated
|
|
||
| auto result = simulate(t0, tmax, dt, model); | ||
|
|
||
| result.print_table(); |
There was a problem hiding this comment.
What is the insight of this output? Maaybe you can output smth more interesting, or at least label the cols.
There was a problem hiding this comment.
Labelled the cols. Not sure what is more interesting than the simulation result
There was a problem hiding this comment.
The labels don't change the fact that the output is way too wide. Right now, it's just pointless because you can't see anything anyway due to the formatting, just a lot of numbers. So you might as well just remove the example entirely :D
The examples should be used to demonstrate the advantage or the function of the approaach
You can also create your own time series from the results.
Maybe plot the percentage of infeted in each region? (interpolate the time points before?)
| { | ||
|
|
||
| /******************** | ||
| * define the model * |
There was a problem hiding this comment.
there is no readme file in this dir.
I think it is important to note there, that is just an implementation of a implicit mobility scheme and that we also have a generic mobility scheme with explicit mobility (the graph-ode) for each of the available ODE models.
There was a problem hiding this comment.
I added the readme file, but only with the two short references to the example and documentation. I also added the reference to the Graph-ODE model to the documentation as I think it should rather belong there.
There was a problem hiding this comment.
"This directory contains a metapopulation model implementation based on an ODE formulation. "
Please add there, what the difference to the graph ODE metapopulation formulation is.
(indirect mobility, impact from others in the force of incfection)
There was a problem hiding this comment.
I'm still wondering. We actually wanted to get rid of the README's for every model completely. Then decided to use them only to reference to the actual documentation. Why don't we just collect all information in one place?
| model.parameters.set<mio::oseirmetapop::TimeInfected<>>(8.097612257); | ||
| model.parameters.set<mio::oseirmetapop::TransmissionProbabilityOnContact<>>(0.07333); | ||
|
|
||
| Construct an ``Eigen::MatrixXd`` of size :math:`n_{regions} \times n_{regions}` which describes the fraction of individuals commuting from one region to another. The matrix should satify the sum of each row equal to 1.0, e.g.: |
There was a problem hiding this comment.
Theres not real "commuting from one region to another" in this model
There was a problem hiding this comment.
That's true, but it still is what we try to model.
There was a problem hiding this comment.
No, we model the impact from commuters in the transmission process in other patches. We do not model the commuting from one region to another :)
There was a problem hiding this comment.
We model the impact of that fraction commuting from one region to another
Co-authored-by: Henrik Zunker <69154294+HenrZu@users.noreply.github.com>
….com:SciCompMod/memilio into 1003-implement-ode-seir-metapopulation-model
| model.parameters.set<mio::oseirmetapop::TimeInfected<>>(8.097612257); | ||
| model.parameters.set<mio::oseirmetapop::TransmissionProbabilityOnContact<>>(0.07333); | ||
|
|
||
| Construct an ``Eigen::MatrixXd`` of size :math:`n_{regions} \times n_{regions}` which describes the fraction of individuals commuting from one region to another. The matrix should satify the sum of each row equal to 1.0, e.g.: |
There was a problem hiding this comment.
No, we model the impact from commuters in the transmission process in other patches. We do not model the commuting from one region to another :)
Changes and Information
Please briefly list the changes (main added features, changed items, or corrected bugs) made:
Replacement for #1128
If need be, add additional information and what the reviewer should look out for in particular:
Merge Request - Guideline Checklist
Please check our git workflow. Use the draft feature if the Pull Request is not yet ready to review.
Checks by code author
Checks by code reviewer(s)