Using keys to preserve values between reloads #8056

aliabid94 · 2024-04-17T22:15:11Z

Added the concept of a key to a Block, such that any Block that has the same key between re-renders has its value preserved between renders. Will be needed specifically for render PR.

Implemented in dev mode by automatically assigning every Block a generated key and giving similarly positioned blocks have the same key across renders. See below how re-renders do not change the value of the components.

BEFORE:

AFTER:

gradio-pr-bot · 2024-04-17T22:15:43Z

🪼 branch checks and previews

•	Name	Status	URL
	Spaces	ready!	Spaces preview
	Website	ready!	Website preview
	Storybook	ready!	Storybook preview
🦄	Changes	detected!	Details

Install Gradio from this PR

pip install https://gradio-builds.s3.amazonaws.com/fe9003186dce675bcf9fd94a892b9bcfb1fae9f2/gradio-4.27.0-py3-none-any.whl

Install Gradio Python Client from this PR

pip install "gradio-client @ git+https://github.com/gradio-app/gradio@fe9003186dce675bcf9fd94a892b9bcfb1fae9f2#subdirectory=client/python"

gradio-pr-bot · 2024-04-17T22:15:52Z

🦄 change detected

This Pull Request includes changes to the following packages.

Package	Version
`@gradio/app`	`minor`
`@gradio/client`	`minor`
`gradio`	`minor`

Maintainers can select this checkbox to manually select packages to update.

With the following changelog entry.

Using keys to preserve values between reloads

Maintainers or the PR author can modify the PR title to modify this entry.

Something isn't right?

Maintainers can change the version label to modify the version bump.
If the bot has failed to detect any changes, or if this pull request needs to update multiple packages to different versions or requires a more comprehensive changelog entry, maintainers can update the changelog file directly.

…/gradio into assign_keys_to_reload

freddyaboulton · 2024-04-19T16:35:41Z

gradio/blocks.py

+        if self.dev_mode:
+            for block in self.blocks.values():
+                if block.key is None:
+                    block.key = f"__{block._id}__"


Can we make the id a hash of the component's config? That way it will be unique across re-renders unless something about the component changed and we don't need to do the reassign_keys logic.

So what you're suggesting is that we use the hash of the config to decide what components are considered identical across re-renders. While we could use that logic in utils.reassign_keys to consider what components are equivalent, it's gonna be problematic because it's very likely two components have the exact same config, e.g.

with gr.Blocks(): a = gr.Textbox() b = gr.Textbox()

I think it makes more sense to rely on the position and type of components to assign identity. The logic in utils.reassign_keys can always be improved but keep in mind this is only for dev mode refreshes - in actual render blocks, we will require users to explicitly assign keys to maintain identity.

I see your point about duplicate keys but is that a concern for reload mode? Even if auto-generated ids collide, the component fetched in the frontend's restored_keyed_values would be correct since the config should determine the state of the component.

The problem with using the position to determine equivalence is that if you insert a component "in between" or "before" already existing components, then the front-end state would get wiped.

This PR is already an improvement of what we currently do but wondering if we can create a better heuristic for equivalence from the get-go.

Even if auto-generated ids collide, the component fetched in the frontend's restored_keyed_values would be correct since the config should determine the state of the component.

I don't think this is the case. If initially I have:

with gr.Blocks(): boxA = gr.Textbox() boxB = gr.Textbox()

and I type something into boxA, and then I change boxA to:

with gr.Blocks(): boxA = gr.Textbox(label="A") boxB = gr.Textbox()

then the value that was in A would get moved to B, since that's the best match by config logic.

This PR is already an improvement of what we currently do but wondering if we can create a better heuristic for equivalence from the get-go.

Sure, I think we can iterate on reassign_keys logic in other PRs, but since its only used in dev mode I think this can be a decent starting point.

Ok sounds good!

pngwn · 2024-04-19T19:28:05Z

js/app/src/Blocks.svelte

+	const restore_keyed_values = (): void => {
+		let component_values_by_key: Record<string | number, ComponentMeta> = {};
+		old_components.forEach((component) => {
+			if (component.key) {
+				component_values_by_key[component.key] = component;
+			}
+		});
+		components.forEach((component) => {
+			if (component.key) {
+				const old_component = component_values_by_key[component.key];
+				if (old_component) {
+					component.props.value = old_component.props.value;
+				}
+			}
+		});
+	};
+


Do you think it would be possible to do all of this in the init module? I think we could expose an update_components function where we could pass in components, and the old / new merging could be handled inside that module with all of the other component updates.

sure, moved to init.ts

Sorry I didn't mean just move the function but rather the old component state could be tracked in the init logic.

We already have the previous component state. If we added an update method that took the new component / deps / layout, we could easily perform the logic all inside of the update method without needing to track the previous state outside of that module.

This way we could also perhaps avoid some work on every component change and instead only do what is necessary (this may be minimal if anything).

Gotcha, makes sense! Implemented this in #8110 which targets this PR.

freddyaboulton · 2024-04-22T17:09:48Z

gradio/utils.py

@@ -155,7 +148,11 @@ def alert_change(self):
        self.change_event.set()

    def swap_blocks(self, demo: Blocks):
+        old_blocks = self.running_app.blocks
        super().swap_blocks(demo)


I think we need to take care of adding max_file_size from the old blocks here as well:

demo.max_file_size = old_blocks.max_file_size

ok, handled in BaseReloader.swap_blocks

freddyaboulton

Thanks for the great PR @aliabid94 !

One remaining issue I noticed is that this breaks custom component reload mode. I created a new component in js/preview/test (needs to be here until the next release because of #8066) and ran dev mode. Changes in the python file were not reflected in the UI. Confirmed it does work on main though.

pngwn · 2024-04-24T16:56:31Z

I can also reproduce what @freddyaboulton is seeing.

aliabid94 · 2024-04-24T23:24:38Z

Okay I believe I fixed the maximum 6 refreshes bug and also the bug @freddyaboulton mentioned by simply closing the heartbeat connection on refreshes. Should be ready!

freddyaboulton

Tested and works well @aliabid94 🔥 Nice

freddyaboulton · 2024-04-25T17:01:31Z

pnpm-lock.yaml

@@ -1478,6 +1478,25 @@ importers:
        specifier: ^3.28.0
        version: 3.28.0

+  js/preview/test/mycomponent/frontend:


Revert this change please

abidlabs · 2024-04-25T17:44:40Z

Didn't do a deep review but it looks like the issues that I noticed are fixed. Nice!!

As discussed previously, would be cool to talk about how values are preserved:

…/gradio into assign_keys_to_reload

changes

f8dedb6

aliabid94 requested review from dawoodkhan82, pngwn, hannahblair and freddyaboulton April 17, 2024 22:15

add changeset

53a2e2c

Ali Abid and others added 7 commits April 18, 2024 14:07

changes

cb4c3d0

add changeset

7e4ae69

changes

2fe9c43

Merge branch 'assign_keys_to_reload' of https://github.com/gradio-app…

da3bb03

…/gradio into assign_keys_to_reload

changes

39decbf

changes

16c9978

Merge remote-tracking branch 'origin' into assign_keys_to_reload

dd146e3

freddyaboulton reviewed Apr 19, 2024

View reviewed changes

pngwn reviewed Apr 19, 2024

View reviewed changes

Ali Abid added 2 commits April 21, 2024 15:24

changes

d67287e

changes

16c5c69

freddyaboulton reviewed Apr 22, 2024

View reviewed changes

freddyaboulton mentioned this pull request Apr 23, 2024

Pass Error status in /dev/reload stream #8106

Merged

aliabid94 requested a review from freddyaboulton April 23, 2024 22:48

Ali Abid added 2 commits April 23, 2024 15:50

changes

8a01f3d

changes

96c1414

freddyaboulton reviewed Apr 24, 2024

View reviewed changes

abidlabs and others added 3 commits April 24, 2024 12:48

Merge branch 'main' into assign_keys_to_reload

591a1b6

changes

03e5c31

add changeset

45b4542

aliabid94 requested review from pngwn and freddyaboulton April 25, 2024 15:45

freddyaboulton approved these changes Apr 25, 2024

View reviewed changes

Ali Abid added 5 commits April 25, 2024 10:46

rev pn

6671a13

Merge branch 'assign_keys_to_reload' of https://github.com/gradio-app…

ac2f20a

…/gradio into assign_keys_to_reload

changes

5bb7bc5

changes

829d3db

changes

fe90031

aliabid94 merged commit 2e469a5 into main Apr 25, 2024
7 of 8 checks passed

aliabid94 deleted the assign_keys_to_reload branch April 25, 2024 18:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Using keys to preserve values between reloads #8056

Using keys to preserve values between reloads #8056

aliabid94 commented Apr 17, 2024

gradio-pr-bot commented Apr 17, 2024 •

edited

Loading

gradio-pr-bot commented Apr 17, 2024 •

edited

Loading

Something isn't right?

freddyaboulton Apr 19, 2024

aliabid94 Apr 21, 2024

freddyaboulton Apr 22, 2024

aliabid94 Apr 23, 2024

freddyaboulton Apr 24, 2024

pngwn Apr 19, 2024

aliabid94 Apr 21, 2024

pngwn Apr 22, 2024

aliabid94 Apr 24, 2024

freddyaboulton Apr 22, 2024

aliabid94 Apr 23, 2024

freddyaboulton left a comment

pngwn commented Apr 24, 2024

aliabid94 commented Apr 24, 2024

freddyaboulton left a comment

freddyaboulton Apr 25, 2024

abidlabs commented Apr 25, 2024 •

edited

Loading

Using keys to preserve values between reloads #8056

Using keys to preserve values between reloads #8056

Conversation

aliabid94 commented Apr 17, 2024

gradio-pr-bot commented Apr 17, 2024 • edited Loading

🪼 branch checks and previews

gradio-pr-bot commented Apr 17, 2024 • edited Loading

🦄 change detected

This Pull Request includes changes to the following packages.

With the following changelog entry.

Something isn't right?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

freddyaboulton left a comment

Choose a reason for hiding this comment

pngwn commented Apr 24, 2024

aliabid94 commented Apr 24, 2024

freddyaboulton left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abidlabs commented Apr 25, 2024 • edited Loading

gradio-pr-bot commented Apr 17, 2024 •

edited

Loading

gradio-pr-bot commented Apr 17, 2024 •

edited

Loading

abidlabs commented Apr 25, 2024 •

edited

Loading