1132 Rename public members of IDE model #1173

annawendler · 2025-01-07T16:10:26Z

Changes and Information

m_transitions was renamed to flows
m_populations was renamed to populations
m_total_confirmed_cases was renamed to total_confirmed_cases
Arguments of model constructor were partially renamed for better understanding

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 for new functionality has been added (Doxygen in the code and Markdown files if necessary)
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).

Closes #1132

codecov · 2025-01-07T16:45:13Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.97%. Comparing base (0c4965a) to head (9a321fc).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1173      +/-   ##
==========================================
- Coverage   96.97%   96.97%   -0.01%     
==========================================
  Files         148      148              
  Lines       13718    13714       -4     
==========================================
- Hits        13303    13299       -4     
  Misses        415      415

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

annawendler · 2025-01-08T12:21:51Z

I changed the name m_transitions to flows because it is in line with our simulation functions where we also use the term flow. The term transition (e.g. in transition_idx) still occurs in some places when we are referring to the transitions themselves and not to the number of individuals transitioning from one compartment to another.

lenaploetzke

Nice work! Please consider especially to rename the InfectionTransisitons and the TransitionDistributions consistent to the flows TimeSeries.

lenaploetzke · 2025-01-09T10:39:44Z

cpp/models/ide_secir/model.h

-        m_transitions; ///< TimeSeries containing points of time and the corresponding number of transitions for every
-    // AgeGroup.
-    TimeSeries<ScalarType> m_populations; ///< TimeSeries containing points of time and the corresponding number of
+        flows; ///< TimeSeries containing points of time and the corresponding number of individuals transitioning from


flows and InfectionTransitions and Transitiondistributions are no longer linked by name. Do you think that's bad? I dont know how to feel about this.

lenaploetzke · 2025-01-09T10:41:25Z

cpp/examples/ide_initialization.cpp

@@ -84,7 +84,7 @@ int main(int argc, char** argv)
            init.add_time_point(init.get_last_time() + dt,
                                Vec::Constant((int)mio::isecir::InfectionTransition::Count, 1. * dt));


Eventually adapt also the num_transitions in the ide examples and name them num_flows? Also in the tests? On the other hand this is consistent with InfectionTransition.. I dont know

lenaploetzke · 2025-01-09T10:44:31Z

cpp/models/ide_secir/model.h

-    * In the TimeSeries we store a vector for each time point. In this vector we store the different 
-    * #InfectionTransition%s for every AgeGroup.
+    * In the TimeSeries we store a vector for each time point. In this vector we store values for the different 
+    * InfectionTransitions for every AgeGroup.


Why not #InfectionTransition%s?

lenaploetzke · 2025-01-09T10:46:09Z

cpp/examples/ide_initialization.cpp

@@ -84,7 +84,7 @@ int main(int argc, char** argv)
            init.add_time_point(init.get_last_time() + dt,
                                Vec::Constant((int)mio::isecir::InfectionTransition::Count, 1. * dt));
        }
-        model.m_transitions = init;


Can you adjust in the example ide_secir lines 78+80+123+124 and in example ide_secir_ageres 64 - 69 + 124-125?

lenaploetzke · 2025-01-09T10:46:40Z

cpp/examples/ide_initialization.cpp

@@ -84,7 +84,7 @@ int main(int argc, char** argv)
            init.add_time_point(init.get_last_time() + dt,
                                Vec::Constant((int)mio::isecir::InfectionTransition::Count, 1. * dt));
        }
-        model.m_transitions = init;
+        model.flows = init;
    }


Please also adapt the README (with m_populations). And maybe transitions to flows.

lenaploetzke · 2025-01-09T10:49:48Z

cpp/models/ide_secir/model.cpp

-    if (!((size_t)m_transitions.get_num_elements() == (size_t)InfectionTransition::Count * m_num_agegroups)) {
-        log_error("A variable given for model construction is not valid. Number of elements in transition vector "
+    if (!((size_t)flows.get_num_elements() == (size_t)InfectionTransition::Count * m_num_agegroups)) {
+        log_error("A variable given for model construction is not valid. Number of elements in vector of flows"


Suggested change

log_error("A variable given for model construction is not valid. Number of elements in vector of flows"

log_error("A variable given for model construction is not valid. Number of elements in vector of flows."

renaming

28d6a77

annawendler linked an issue Jan 7, 2025 that may be closed by this pull request

Rename public members in IDE model #1132

Open

some adaptations

9a321fc

annawendler requested a review from lenaploetzke January 8, 2025 13:48

lenaploetzke reviewed Jan 9, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

1132 Rename public members of IDE model #1173

1132 Rename public members of IDE model #1173

annawendler commented Jan 7, 2025 •

edited

Loading

codecov bot commented Jan 7, 2025 •

edited

Loading

annawendler commented Jan 8, 2025

lenaploetzke left a comment

lenaploetzke Jan 9, 2025

lenaploetzke Jan 9, 2025

lenaploetzke Jan 9, 2025

lenaploetzke Jan 9, 2025

lenaploetzke Jan 9, 2025

lenaploetzke Jan 9, 2025

		@@ -84,7 +84,7 @@ int main(int argc, char** argv)
		init.add_time_point(init.get_last_time() + dt,
		Vec::Constant((int)mio::isecir::InfectionTransition::Count, 1. * dt));

	log_error("A variable given for model construction is not valid. Number of elements in vector of flows"
	log_error("A variable given for model construction is not valid. Number of elements in vector of flows."

1132 Rename public members of IDE model #1173

Are you sure you want to change the base?

1132 Rename public members of IDE model #1173

Conversation

annawendler commented Jan 7, 2025 • edited Loading

Changes and Information

Merge Request - Guideline Checklist

Checks by code author

Checks by code reviewer(s)

codecov bot commented Jan 7, 2025 • edited Loading

Codecov Report

annawendler commented Jan 8, 2025

lenaploetzke left a comment

Choose a reason for hiding this comment

lenaploetzke Jan 9, 2025

Choose a reason for hiding this comment

lenaploetzke Jan 9, 2025

Choose a reason for hiding this comment

lenaploetzke Jan 9, 2025

Choose a reason for hiding this comment

lenaploetzke Jan 9, 2025

Choose a reason for hiding this comment

lenaploetzke Jan 9, 2025

Choose a reason for hiding this comment

lenaploetzke Jan 9, 2025

Choose a reason for hiding this comment

annawendler commented Jan 7, 2025 •

edited

Loading

codecov bot commented Jan 7, 2025 •

edited

Loading