1469 Correct nonageresolved indices in IDE-SECIR model #1472
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1472 +/- ##
==========================================
+ Coverage 97.31% 97.39% +0.08%
==========================================
Files 187 189 +2
Lines 16017 16533 +516
==========================================
+ Hits 15587 16103 +516
Misses 430 430 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
reneSchm
left a comment
reneSchm
left a comment
There was a problem hiding this comment.
Very nice documentation for the new test. The only important comment is on line 278 regarding testing interactions between age groups.
cpp/tests/test_ide_secir_ageres.cpp
Outdated
| // Model with whole population in second age group with index 1. Use baseline values scaled by baseline_scaling. | ||
| considered_group = 1; | ||
| std::vector<ScalarType> N_vec_group1 = {0., baseline_scaling * population_baseline}; | ||
| std::vector<ScalarType> deaths_vec_group1 = {0., baseline_scaling * deaths_baseline}; |
There was a problem hiding this comment.
I would have expected that the second test uses both populations to check that there are no unexpected interactions between age groups. With the current setup, any interactions between age groups vanishes (as the "other" age group is always 0), so in theory that could hide indexing errors.
Do you want to cover that case?
There was a problem hiding this comment.
That's right. I adapted the test so that now I have two model with two different age groups (the second as a multiple of the first one). The models differ with respect to their contact matrices. In the first model I only have contacts within each group and in the second model individuals have only contact with the other group. This should cover the relevant cases.
| model_inter.parameters.set<mio::isecir::Seasonality>(0.); | ||
| model_inter.parameters.set<mio::isecir::StartDay>(0); | ||
|
|
||
| // Here we set the contact matrices for both models which is where they differ from each other. |
There was a problem hiding this comment.
Is the LCT Model copyable? If so, you can save a good number of lines by only setting up one model, and then copying it right here.
| EXPECT_NEAR(secihurd_simulated_intra[0][age2], baseline_scaling * secihurd_t0_baseline[i], 1e-8); | ||
| EXPECT_NEAR(secihurd_simulated_intra[1][age1], secihurd_t1_baseline[i], 1e-8); | ||
| EXPECT_NEAR(secihurd_simulated_intra[1][age2], baseline_scaling * secihurd_t1_baseline[i], 1e-8); | ||
| EXPECT_NEAR(secihurd_simulated_inter[0][age1], secihurd_t0_baseline[i], 1e-8); |
There was a problem hiding this comment.
Maybe put a comment (or just a newline between the EXPECTs? Otherwise the inters and intras completely blend together.
| EXPECT_NEAR(secihurd_simulated_intra[0][age1], secihurd_t0_baseline[i], 1e-8); | ||
| EXPECT_NEAR(secihurd_simulated_intra[0][age2], baseline_scaling * secihurd_t0_baseline[i], 1e-8); | ||
| EXPECT_NEAR(secihurd_simulated_intra[1][age1], secihurd_t1_baseline[i], 1e-8); | ||
| EXPECT_NEAR(secihurd_simulated_intra[1][age2], baseline_scaling * secihurd_t1_baseline[i], 1e-8); | ||
| EXPECT_NEAR(secihurd_simulated_inter[0][age1], secihurd_t0_baseline[i], 1e-8); | ||
| EXPECT_NEAR(secihurd_simulated_inter[0][age2], baseline_scaling * secihurd_t0_baseline[i], 1e-8); | ||
| EXPECT_NEAR(secihurd_simulated_inter[1][age1], secihurd_t1_baseline[i], 1e-8); | ||
| EXPECT_NEAR(secihurd_simulated_inter[1][age2], baseline_scaling * secihurd_t1_baseline[i], 1e-8); |
There was a problem hiding this comment.
Why do both models have the same result?
I would have expected different scaling between the inter and intra groups, as the second age group has twice the population as the first. So when only inter-age group interactions happen, I'd expect the first age group double in infection rate (and half for the second) compared to the intra model.
There was a problem hiding this comment.
In both groups we have the same fraction of infected individuals compared to the total population, so the probability of meeting an infected individual is the same for both groups. Because we have the same number of contacts to other individuals for each group in both models, we get the same results. I can add a sentence on that.
There was a problem hiding this comment.
Maybe my intuition was just wrong here. I expected the interaction to work like
S_0' = S_0 * I_1 / N_0
instead of
S_0' = S_0 * I_1 / N_1
So it is probably fine without a comment. Though explaining why sth works in code never hurts.
Changes and Information
In the IDE-SECIR model, two indices were not adapted for multiple age groups. This was corrected here and appropriate test was added to check the correctness of the ageresolved simulation.
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)
Closes #1469