1393 improve parameters io for graph ode based models by MaxBetzDLR · Pull Request #1430 · SciCompMod/memilio

MaxBetzDLR · 2025-11-24T09:35:17Z

Changes and Information

Please briefly list the changes (main added features, changed items, or corrected bugs) made:

Rework current structure to be more flexible and similar between models
Define functions for each type of epidata setter, that works on a single region
Use graph for setting epidata

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

Every addressed issue is linked (use the "Closes #ISSUE" keyword below)
New code adheres to coding guidelines
No large data files have been added (files should in sum not exceed 100 KB, avoid PDFs, Word docs, etc.)
Tests are added for new functionality and a local test run was successful (with and without OpenMP)
Appropriate documentation within the code (Doxygen) for new functionality has been added in the code
Appropriate external documentation (ReadTheDocs) for new functionality has been added to the online documentation
Proper attention to licenses, especially no new third-party software with conflicting license has been added
(For ABM development) Checked benchmark results and ran and posted a local test above from before and after development to ensure performance is monitored.

Checks by code reviewer(s)

Corresponding issue(s) is/are linked and addressed
Code is clean of development artifacts (no deactivated or commented code lines, no debugging printouts, etc.)
Appropriate unit tests have been added, CI passes, code coverage and performance is acceptable (did not decrease)
No large data files added in the whole history of commits(files should in sum not exceed 100 KB, avoid PDFs, Word docs, etc.)
On merge, add 2-5 lines with the changes (main added features, changed items, or corrected bugs) to the merge-commit-message. This can be taken from the briefly-list-the-changes above (best case) or the separate commit messages (worst case).

codecov · 2025-11-25T11:52:59Z

Codecov Report

❌ Patch coverage is 94.48560% with 67 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.30%. Comparing base (edb7428) to head (d6f9906).

Files with missing lines	Patch %	Lines
cpp/models/ode_secirts/parameters_io.h	56.14%	25 Missing ⚠️
cpp/models/ode_secirvvs/parameters_io.cpp	97.11%	13 Missing ⚠️
cpp/models/ode_secirts/parameters_io.cpp	97.50%	10 Missing ⚠️
cpp/models/ode_secirvvs/parameters_io.h	82.69%	9 Missing ⚠️
cpp/models/ode_secir/parameters_io.cpp	96.36%	8 Missing ⚠️
cpp/memilio/io/parameters_io.cpp	91.66%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1430      +/-   ##
==========================================
- Coverage   97.31%   97.30%   -0.01%     
==========================================
  Files         187      187              
  Lines       16018    15820     -198     
==========================================
- Hits        15588    15394     -194     
+ Misses        430      426       -4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…ed-models

HenrZu

Great addition :)
Most of my comments are minor. When fixing them, please keep in mind that they often affect other models as well.
What still concerns me yet is the new h5 file. Have you tried with h5dump to see exactly what has changed? And to what extend?

HenrZu · 2025-12-04T12:52:37Z

cpp/memilio/geography/regions.cpp


 StateId get_state_id(int county)
 {
+    // integer division


Suggested change

// integer division

HenrZu · 2025-12-04T13:07:55Z

cpp/memilio/geography/regions.h

+
+struct EpidataFilenames {
+private:
+    EpidataFilenames(std::string& pydata)


Suggested change

EpidataFilenames(std::string& pydata)

EpidataFilenames(const std::string& pydata)

same for the others below

HenrZu · 2025-12-04T13:08:17Z

cpp/memilio/geography/regions.h

+namespace de
+{
+
+struct EpidataFilenames {


add a documentation above the struct

HenrZu · 2025-12-04T13:09:45Z

cpp/memilio/geography/regions.h

+
+        s.case_data_path        = mio::path_join(pydata, "cases_all_state_age_ma7.json");
+        s.divi_data_path        = mio::path_join(pydata, "state_divi_ma7.json");
+        s.vaccination_data_path = mio::path_join(pydata, "vacc_state_ageinf_ma7.json");


the vaccination data is the same for states and county. intended? if yes, add a comment.

HenrZu · 2025-12-04T13:12:35Z

cpp/memilio/geography/regions.h

+    {
+    }
+
+public:


overall, this is all very static. At least the moving average should be adaptive.
use sth like:
std::string ma_suffix = "_ma" + std::to_string(moving_average) + ".json";
in combination with default moving_average=7.

HenrZu · 2025-12-04T13:38:58Z

cpp/models/ode_secirvvs/parameters_io.cpp

+{
+    BOOST_OUTCOME_TRY(auto&& case_data, mio::read_confirmed_cases_data(path));
+
+    // sort case_data into regions and ignore once with no region associated


Suggested change

// sort case_data into regions and ignore once with no region associated

// sort case_data into regions and ignore ones with no region associated

please also correct in sceir and secirvvs

HenrZu · 2025-12-04T13:39:47Z

cpp/models/ode_secirvvs/parameters_io.cpp

+        }
+        model.populations[{AgeGroup(i), InfectionState::SusceptibleImprovedImmunity}] = num_rec[i];
+        if (set_death) {
+            // in set_confirmed_cases_data initilization, deaths are now set to 0. In order to visualize


Suggested change

// in set_confirmed_cases_data initilization, deaths are now set to 0. In order to visualize

// in set_confirmed_cases_data initialization, deaths are now set to 0. In order to visualize

also use search + replace

HenrZu · 2025-12-04T13:45:47Z

cpp/tests/test_epi_data_io.cpp

    ASSERT_EQ(vacc_data[1].num_vaccinations_refreshed_additional, 3.0);
 }

-TEST(TestEpiData, set_vaccination_data)


why not reuse this test in the secirts test file?

HenrZu · 2025-12-04T13:46:31Z

cpp/tests/test_graph.cpp

-TEST(TestGraph, set_nodes_secir)
-{
-
-    mio::osecir::Parameters<double> params(1);


why delete?

HenrZu · 2025-12-05T07:16:54Z

cpp/memilio/mobility/graph.h

+ * @param[in] end_date Date at the end of the simulation.
+ */
+template <class FP, class Model, class ContactPattern>
+void set_german_holidays(const mio::VectorRange<Node<Model>>& nodes, const mio::Date& start_date,


in generell, we should try to keep this generic and add a overall function set_holidays which allows a country specifier e.g., for Germany.
however, since we only have data for germany at this moment, this is ok :)

MaxBetzDLR added 10 commits October 16, 2025 19:58

Simplify parameters io

1959d3b

data setter redesign

664689f

remake divi data

561f657

fix node range and de namespace

a1d73a7

fixed bugs

d7ce1f1

fix graph functions

c553af3

Adapt documentation for new interfaces

511956b

Fix bugs

d054a5e

Bindings update

0d2adcf

Update tests with big fixes

db7d684

MaxBetzDLR requested a review from HenrZu November 24, 2025 09:35

MaxBetzDLR self-assigned this Nov 24, 2025

MaxBetzDLR added the class::improvement Cleanup that doesn't affect functionality label Nov 24, 2025

MaxBetzDLR linked an issue Nov 24, 2025 that may be closed by this pull request

Improve parameters IO for graph ODE-based models #1393

Open

2 tasks

MaxBetzDLR added 10 commits November 24, 2025 13:54

Fix osecirts tests

0e0c23a

Change test data of export time series

3b3c628

fix bindings

9461e4d

Const string

bfe89f2

Fix bindings

356b84d

Fix tests

d7cf12c

Fix bindings

e5de2e2

pre-commit

4e58516

missing characters

460db72

Add include

15beb32

MaxBetzDLR added 5 commits November 25, 2025 12:53

Convert Population

cade4eb

Merge branch 'main' into 1393-improve-parameters-io-for-graph-ode-bas…

50e5d6f

…ed-models

fix ad types

d188bc9

Merge main

16e43e8

move regions change to followup issue

adcee20

MaxBetzDLR added 3 commits November 27, 2025 19:38

remove redundant code

28dc354

update bindings

c348614

missplaced cast

d6f9906

HenrZu requested changes Dec 5, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

1393 improve parameters io for graph ode based models#1430

1393 improve parameters io for graph ode based models#1430
MaxBetzDLR wants to merge 28 commits intomainfrom
1393-improve-parameters-io-for-graph-ode-based-models

MaxBetzDLR commented Nov 24, 2025 •

edited by HenrZu

Loading

Uh oh!

codecov bot commented Nov 25, 2025 •

edited

Loading

Uh oh!

HenrZu left a comment •

edited

Loading

Uh oh!

HenrZu Dec 4, 2025

Uh oh!

HenrZu Dec 4, 2025

Uh oh!

HenrZu Dec 4, 2025

Uh oh!

HenrZu Dec 4, 2025

Uh oh!

HenrZu Dec 4, 2025

Uh oh!

HenrZu Dec 4, 2025

Uh oh!

HenrZu Dec 4, 2025

Uh oh!

HenrZu Dec 4, 2025

Uh oh!

HenrZu Dec 4, 2025

Uh oh!

HenrZu Dec 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	EpidataFilenames(std::string& pydata)
	EpidataFilenames(const std::string& pydata)

+                  {
+                  }
+              public:

	// sort case_data into regions and ignore once with no region associated
	// sort case_data into regions and ignore ones with no region associated

	// in set_confirmed_cases_data initilization, deaths are now set to 0. In order to visualize
	// in set_confirmed_cases_data initialization, deaths are now set to 0. In order to visualize

Conversation

MaxBetzDLR commented Nov 24, 2025 • edited by HenrZu Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes and Information

Merge Request - Guideline Checklist

Checks by code author

Checks by code reviewer(s)

Uh oh!

codecov bot commented Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

HenrZu left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

MaxBetzDLR commented Nov 24, 2025 •

edited by HenrZu

Loading

codecov bot commented Nov 25, 2025 •

edited

Loading

HenrZu left a comment •

edited

Loading