Skip to content

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

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

xsaschako
Copy link
Member

Changes and Information

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

  • Delete unnecessary simulations and integrate them into the examples.

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

  • 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).

@xsaschako xsaschako linked an issue May 22, 2025 that may be closed by this pull request
@xsaschako xsaschako requested a review from Copilot May 22, 2025 10:46
@xsaschako xsaschako self-assigned this May 22, 2025
Copy link

@Copilot Copilot AI left a 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 configured start_date. Consider using the existing start_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 to personal_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 new abm_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;
Copy link
Preview

Copilot AI May 22, 2025

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.

Suggested change
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*/};
Copy link
Preview

Copilot AI May 22, 2025

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.

@xsaschako xsaschako requested a review from charlie0614 May 22, 2025 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Restructure ABM Examples and Simulations
1 participant