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

(app): Add load_state_dict and state_dict #14100

Merged
merged 20 commits into from
Sep 12, 2022
Merged

(app): Add load_state_dict and state_dict #14100

merged 20 commits into from
Sep 12, 2022

Conversation

tchaton
Copy link
Contributor

@tchaton tchaton commented Aug 8, 2022

What does this PR do?

This PR introduces an API to reload dynamic component trees.

The current strategy is the following. We collect all the states. On reload, if a component state doesn't have its component, we give the responsibility to its closest parent to resolve it and then the cascade of instantiation is done.

class FlowReload(LightningFlow):

    def run(self):
        if not getattr(self, "w", None):
            self.w = WorkReload()
        self.w.run()

    def on_load_state_dict(self, children_states, extras) -> None:
        self.w = WorkReload()

Fixes #<issue_number>

Does your PR introduce any breaking changes? If yes, please list them.

Before submitting

  • Was this discussed/approved via a GitHub issue? (not for typos and docs)
  • Did you read the contributor guideline, Pull Request section?
  • Did you make sure your PR does only one thing, instead of bundling different changes together?
  • Did you make sure to update the documentation with your changes? (if necessary)
  • Did you write any new necessary tests? (not for typos and docs)
  • Did you verify new and existing tests pass locally with your changes?
  • Did you list all the breaking changes introduced by this pull request?
  • Did you update the CHANGELOG? (not for typos, docs, test updates, or minor internal changes/refactors)

PR review

Anyone in the community is welcome to review the PR.
Before you start reviewing, make sure you have read the review guidelines. In short, see the following bullet-list:

  • Is this pull request ready for review? (if not, please submit in draft mode)
  • Check that all items from Before submitting are resolved
  • Make sure the title is self-explanatory and the description concisely explains the PR
  • Add labels and milestones (and optionally projects) to the PR so it can be classified

Did you have fun?

Make sure you had fun coding 🙃

cc @Borda

@tchaton tchaton marked this pull request as draft August 8, 2022 20:52
@github-actions github-actions bot added the app (removed) Generic label for Lightning App package label Aug 8, 2022
@tchaton tchaton marked this pull request as ready for review August 9, 2022 14:46
@tchaton tchaton changed the title PoC: Add state_dict and load_state_dict PoC: Add on_load_state_dict and on_save_state_dict Aug 9, 2022
@tchaton tchaton changed the title PoC: Add on_load_state_dict and on_save_state_dict (app): Add on_load_state_dict and on_save_state_dict Aug 9, 2022
@tchaton tchaton added this to the app:0.6 milestone Aug 9, 2022
src/lightning_app/core/flow.py Outdated Show resolved Hide resolved
src/lightning_app/core/flow.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@lantiga lantiga left a comment

Choose a reason for hiding this comment

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

A few comments on capturing a "resumable state" from Work.

src/lightning_app/core/flow.py Outdated Show resolved Hide resolved
src/lightning_app/core/work.py Show resolved Hide resolved
src/lightning_app/utilities/app_helpers.py Show resolved Hide resolved
@tchaton tchaton force-pushed the on_load_checkpoint branch from 2deac5a to 7c50a5f Compare August 10, 2022 14:22
@mergify mergify bot removed the has conflicts label Aug 10, 2022
@tchaton tchaton force-pushed the on_load_checkpoint branch from fe8e2f6 to bff2986 Compare August 10, 2022 14:25
@tchaton tchaton requested a review from lantiga August 10, 2022 15:00
@mergify mergify bot added has conflicts and removed ready PRs ready to be merged labels Aug 11, 2022
@Borda Borda requested review from carmocca and awaelchli August 17, 2022 12:27
@mergify mergify bot added ready PRs ready to be merged and removed has conflicts ready PRs ready to be merged labels Aug 18, 2022
@hhsecond hhsecond modified the milestones: app:0.6, app:0.6.x Aug 23, 2022
@Borda
Copy link
Member

Borda commented Sep 7, 2022

@tchaton, mind checking the remaining comments?

@mergify mergify bot added has conflicts and removed ready PRs ready to be merged labels Sep 12, 2022
@mergify mergify bot added ready PRs ready to be merged and removed has conflicts ready PRs ready to be merged labels Sep 12, 2022
@Borda Borda enabled auto-merge (squash) September 12, 2022 17:53
@Borda Borda merged commit 80e2f09 into master Sep 12, 2022
@Borda Borda deleted the on_load_checkpoint branch September 12, 2022 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app (removed) Generic label for Lightning App package ready PRs ready to be merged
Projects
No open projects
Status: Done
Status: Done
Development

Successfully merging this pull request may close these issues.

8 participants