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

experiment should only need one wait_on_done method #2037

Open
shchen-idmod opened this issue Mar 15, 2023 · 5 comments
Open

experiment should only need one wait_on_done method #2037

shchen-idmod opened this issue Mar 15, 2023 · 5 comments
Assignees
Labels
Discuss Discuss with group

Comments

@shchen-idmod
Copy link
Collaborator

Currently we have to use both at same time with wait_on_done and wait_until_done with False case. This is useful with dry_run=True.
We should change to allow only one wait.

@devclinton devclinton added the Discuss Discuss with group label Aug 25, 2023
@devclinton
Copy link
Member

Let's document what changes should be done here, discuss the impact, how we manage downstream impacts, communication etc

@devclinton
Copy link
Member

Steps to do the fix:

  1. First we decide which wait we want? Is it wait_until_done or wait_on_done?
  2. After decided, remove that option from the run function arguments. In the below I assume the wait_on_done is to be removed.

You then check the run_opts and if wait_on_done exists, tell the user it will be deprecated on a release scheduled for X(one month out)?

    def run(self, wait_until_done: bool = False, platform: 'IPlatform' = None, regather_common_assets: bool = None,
            wait_on_done_progress: bool = True,
            **run_opts) -> NoReturn:
        """
        Runs an experiment on a platform.

        Args:
            wait_until_done: Whether we should wait on experiment to finish running as well. Defaults to False
            platform: Platform object to use. If not specified, we first check object for platform object then the current context
            regather_common_assets: Triggers gathering of assets for *existing* experiments. If not provided, we use the platforms default behaviour. See platform details for performance implications of this. For most platforms, it should be ok but for others, it could decrease performance when assets are not changing.
              It is important to note that when using this feature, ensure the previous simulations have finished provisioning. Failure to do so can lead to unexpected behaviour
            wait_on_done_progress: Should experiment status be shown when waiting
            **run_opts: Options to pass to the platform

        Returns:
            None
        """
        if 'wait_on_done' in run_opts:
            warnings.warn("wait_on_done will be deprecated in the next release. Please change to <>")
  1. A release should be done with that warning. The user's should also get an email about the change. Where possible, we should make PRs to known areas(emodpy examples)
  2. Setup a scheduling meeting 1 week before planned final released(1-month out). On that release, remove the warning and quite supporting the old option. We still check that option and then tell user we are now ignoring it for another few releases

@devclinton
Copy link
Member

devclinton commented Sep 13, 2023

Ensure we inform @tfischle-idmod @jonathanhhb @stitova-idm and other stakeholders of this change. Ensure emodpy examples are updated. Check in with existing researchers as well(most likely these are all one line errors, but having documentation on how it will be fixes is useful)

e = Experiment()
e.run(wait_until_on=True)

@devclinton
Copy link
Member

@shchen-idmod
Copy link
Collaborator Author

shchen-idmod commented Jan 24, 2024

there are 265 files for "wait_until_done=" and 105 files for "wait_on_done=" cross InstituteforDiseaseModeling. I think we should drop wait_on_done. For first time change, I am just going to print warning message

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discuss Discuss with group
Projects
None yet
Development

No branches or pull requests

3 participants