-
Notifications
You must be signed in to change notification settings - Fork 19
1284 restructure abm examples and simulations #1287
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR removes redundant ABM simulation targets and consolidates simulation setup into the example, updating the example’s initialization logic and renaming the primary ABM example target.
- Deleted standalone ABM simulation executables from
cpp/simulations/CMakeLists.txt - Enhanced
abm.cppto seed infections probabilistically and set mask compliance at person and location levels - Renamed and updated the ABM example target in
cpp/examples/CMakeLists.txt
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| cpp/simulations/CMakeLists.txt | Removed abm_simulation and abm_braunschweig executables |
| cpp/examples/abm.cpp | Replaced uniform random infection assignment with parameterized probability, added mask compliance logic |
| cpp/examples/CMakeLists.txt | Renamed abm_history_example to abm_example and updated target |
Comments suppressed due to low confidence (3)
cpp/examples/abm.cpp:150
- Hardcoding
TimePoint(0)may misalign the infection start with the configuredstart_date. Consider using the existingstart_datevariable to ensure consistency.
model.parameters, mio::abm::TimePoint(0), state));
cpp/examples/abm.cpp:143
- [nitpick] The variable name
prngis terse; consider renaming it topersonal_rngor similar for clarity about its purpose.
auto prng = mio::abm::PersonalRandomNumberGenerator(person);
cpp/examples/CMakeLists.txt:103
- Ensure that any documentation, scripts, or CI configurations referencing
abm_history_exampleare updated to use the newabm_exampletarget.
add_executable(abm_example abm.cpp)
| for (auto& person : model.get_persons()) { | ||
| auto prng = mio::abm::PersonalRandomNumberGenerator(person); | ||
| //some % of people are infected, large enough to have some infection activity without everyone dying | ||
| auto pct_infected = 0.05; |
Copilot
AI
May 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Extract the magic number 0.05 into a named constant (e.g., initial_infection_probability) to improve readability and make future adjustments easier.
| auto pct_infected = 0.05; | |
| auto pct_infected = initial_infection_probability; |
| } | ||
|
|
||
| //equal chance of (moderate) mask refusal and (moderate) mask eagerness | ||
| auto pct_compliance_values = std::array{0.05 /*0*/, 0.2 /*0.25*/, 0.5 /*0.5*/, 0.2 /*0.75*/, 0.05 /*1*/}; |
Copilot
AI
May 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Consider moving these compliance weight values into a named constant or configuration to clarify their meaning and avoid inline literals.
Changes and Information
Please briefly list the changes (main added features, changed items, or corrected bugs) made:
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)