Skip to content
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

low level logic to disable modules #799

Merged
merged 6 commits into from
Feb 8, 2023
Merged

low level logic to disable modules #799

merged 6 commits into from
Feb 8, 2023

Conversation

AdrienLeGuillou
Copy link
Member

WIP

  • makes a named list of module on initialization
  • add a "utility module" afeter updater_net that will disable modules listed in param$.modules.to.disable
  • this param is cleared after the modules are disabled
  • meant to be used with scenarios to chose at what time to remove what modules

@AdrienLeGuillou
Copy link
Member Author

Current state:

  • the core elements are working
  • user facing interface to finalize

after discussion with @smjenness, revised specs:

  • do not define the .modules.to.remove through scenarios as initially planned
  • pass the modules to remove to control.net and not as params
  • (no changes to the internal code though)

@AdrienLeGuillou AdrienLeGuillou marked this pull request as ready for review January 11, 2023 12:20
@AdrienLeGuillou
Copy link
Member Author

This PR creates a list of modules in the initialization step to allow removing some of them later on in the simulation.

The new end.horizon control will trigger the removal of a module set at a given time step

To trigger an end horizon at step 204 that would remove the arrival and infection modules, one would do:

control.net(
  ...,
  end.horizon = list(at = 204, modules = c("arrivals.FUN", "infections.FUN"),
  ...
)

At initiation, a check verifies that the modules to be removed are actually in the module list and that the time step of trigger is after the start of simulation.

@smjenness: was that the type of user facing interface you wanted? I can easily change it if needed

Copy link
Collaborator

@smjenness smjenness left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really great, Adrien! My only minor comment is that the unit testing could be strengthened. For example, if we drop the network resimulation module at time step 5, then we should expect the number of edges to stay constant from 5 to nsteps. That is something you can test with the netsim object with get_nwstats. Perhaps think about that too for the recovery module. This will just further solidify that the modules are dropped as expected. Thanks!

@smjenness
Copy link
Collaborator

@AdrienLeGuillou : as per our conversation earlier today, I am merging in this PR now so that colleagues can start working with the functionality on the main branch, but please do open a new PR to add extra unit tests (and potentially move that nwstats calculation code from the network resimulation module to the prevalence module). Thanks!

@smjenness smjenness merged commit c41ae36 into main Feb 8, 2023
@smjenness smjenness deleted the flex_modules branch February 8, 2023 18:24
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.

2 participants