-
Notifications
You must be signed in to change notification settings - Fork 16
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.cpp
to 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_date
variable to ensure consistency.
model.parameters, mio::abm::TimePoint(0), state));
cpp/examples/abm.cpp:143
- [nitpick] The variable name
prng
is terse; consider renaming it topersonal_rng
or 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_example
are updated to use the newabm_example
target.
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; |
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; |
Copilot uses AI. Check for mistakes.
} | ||
|
||
//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*/}; |
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.
Copilot uses AI. Check for mistakes.
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)