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

[BUG] Dependent callbacks are not waited and executed in order #1519

Closed
fraghag opened this issue Jan 7, 2021 · 14 comments · Fixed by #2344
Closed

[BUG] Dependent callbacks are not waited and executed in order #1519

fraghag opened this issue Jan 7, 2021 · 14 comments · Fixed by #2344
Assignees
Labels
bug something broken

Comments

@fraghag
Copy link

fraghag commented Jan 7, 2021

Describe your context
I have a problem with chained callbacks where the last callback is called twice from one UI change. In the documentation I found this
https://dash.plotly.com/basic-callbacks
where it states under “Dash App With Chained Callbacks” that:

The final callback displays the selected value of each component. If you change the value of the countries RadioItems component, Dash will wait until the value of the cities component is updated before calling the final callback. This prevents your callbacks from being called with inconsistent state like with "America" and "Montréal" .

But when changing between America and Canada in the example on this page I can see that it first updates the text with the country and then the city. So when changing from America to Canada I can see “New York City is a city in Canada” very briefly which according to the documentation should not happen. I also ran this example locally with debugging and verified that the callback updating the text is run twice whenever I change the country.

dash                               1.18.0
dash-bootstrap-components          0.10.7
dash-core-components               1.14.0
dash-html-components               1.1.1
dash-renderer                      1.8.3
dash-ri-components                 2.1.0
dash-table                         4.11.1

Describe the bug
Callback A and callback B has resource A as a dependency, callback B is also dependent on the output of callback A.
When changing resource A, then callback A can be run before callback B is finished, and then runs again when callback A finishes since callback A updates another of callback B's input.

image

Expected behavior

When chaining callbacks, callback B should wait for callback A to finish, if callback B is dependent on the output of callback A.

@alexcjohnson alexcjohnson added the bug something broken label Jan 7, 2021
@alexcjohnson
Copy link
Collaborator

Thanks @fraghag - I see it too and you're absolutely right that this is a bug. We'll have to poke around and see if this is a regression or simply a case that for some reason we've never handled correctly.

@francescobodria
Copy link

I have the same problem with an app that I am developing.
I have two chained callbacks, one change the values of several sliders (10) together and the other one implements a function on these sliders values. The problem is that I need to evaluate the function when all the slider values have changed but I found that it fires up ten times, once every time a single slider value is updated. This causes a problem since it fires up with only partially changed values and raises errors.

@eff-kay
Copy link
Contributor

eff-kay commented May 6, 2021

Hey everyone, I have started working on this issue.

First of all, can someone confirm by looking at the following screen recording if I am reproducing the issue correctly.

https://www.loom.com/share/daaf5d30f76e455aa46705829fdd739d

@alexcjohnson @Marc-Andre-Rivet @fraghag

@eff-kay
Copy link
Contributor

eff-kay commented May 6, 2021

Bringing the discussion from slack in here.

Following is the graph created for the chained callbacks app, where (CoV = CountriesValue, CiV= CitiesValue, CiO = CitiesOptions, DiC = DisplayChildren, cb = callback ). I have left out CountriesOptions (CoO) because that is not relevant here.

Screen Shot 2021-05-06 at 10 40 35 AM

Following are the steps performed, when CountriesValue is updated.

  1. Call the CitiesOptions callback, and DisplayChildren callback.
  2. CitiesOptions calls the CitiesValue callback.
  3. CitiesValue calls the DisplayChildren again.

DisplayChildren is called twice, the first one renders a stale value. But eventually, the correct value is displayed.

@eff-kay
Copy link
Contributor

eff-kay commented May 6, 2021

@alexcjohnson commented the following

Ah OK, I see what you’re saying Faizan. I think your interpretation is correct, but yeah we may need to specifically handle several cases differently:

  • if a callback outputs one prop that then internally to a component changes another prop (ie changing options causes value to be changed)

    • not really sure there can be a general solution for that… at the renderer level we don’t know what internal linkages a component has.
  • if a callback outputs children, and inside one of those components there is another callback output

    • we could look at the paths involved and have the parent callback block the child callback. This is tricky though because there can be cases we use this kind of callback intentionally to generate circularity (that the user then needs to carefully manage to avoid infinite loops) and we wouldn’t want that case to completely lock up.

@eff-kay
Copy link
Contributor

eff-kay commented May 6, 2021

The fix that I have right now is the second one, i.e. CountriesValue upon update, initiates two callbacks (CitiesOptions, and DisplayValue).

I traverse the CitiesOptions callback path to see if it calls DisplayValue at any point. If yes, then discard DisplayValue. from the children of CountriesValue

I don't think the first one is the cause of the issue here, if I am understanding it correctly. The issue will only occur if it's the second case. i.e. a callback is called twice, and one of them returns a stale value because it's called too soon.

@eff-kay
Copy link
Contributor

eff-kay commented May 6, 2021

In summary the two options that @wbrgss mentioned seem to be the solutions here.

  1. Prune the extraneous callback, which is what I am doing right now.
  2. Block/hold for dependencies, this will require some sort of dependency resolution.

@isCopyman
Copy link

same problem

@cdolfi
Copy link

cdolfi commented May 5, 2022

is there any update on this?

@eff-kay
Copy link
Contributor

eff-kay commented May 6, 2022

cc @alexcjohnson.

@groegercesg
Copy link

Bumping again, I’m encountering this bug so very interested in getting a fix

@eff-kay
Copy link
Contributor

eff-kay commented Jul 6, 2022

Hi @groegercesg, this is in the queue, the fix has been finalized, just needs some final testing and rebasing before it can be merged.

@fraghag
Copy link
Author

fraghag commented Nov 7, 2022

Hi @eff-kay! Any news on this bug fix?

@eff-kay
Copy link
Contributor

eff-kay commented Nov 7, 2022

@fraghag sorry we got busy with the DE5 release. Now that the release is done, I can schedule some time to get this through. Ideally in the next few weeks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something broken
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants