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

Treemap in infinite loop when valueAccessor returns zero #642

Closed
wylieconlon opened this issue Apr 21, 2020 · 4 comments · Fixed by #658 or elastic/kibana#64759
Closed

Treemap in infinite loop when valueAccessor returns zero #642

wylieconlon opened this issue Apr 21, 2020 · 4 comments · Fixed by #658 or elastic/kibana#64759
Assignees
Labels
bug Something isn't working :partition Partition/PieChart/Donut/Sunburst/Treemap chart related released Issue released publicly

Comments

@wylieconlon
Copy link

This is a bug in the treemap only, not in the sunburst chart. To reproduce:

  1. Modify one of the 2-layer treemap examples to sometimes return 0 in the valueAccessor function, such as valueAccessor={(d: Datum) => Math.random() < 0.7 ? d.exportVal as number : 0}
  2. Refresh the storybook
  3. You will see that the chart throws an infinite loop:
RangeError: Maximum call stack size exceeded
    at Function.get [Symbol.species] ()
    at Array.concat ()
    at push../src/chart_types/partition_chart/layout/utils/treemap.ts.__spread 

When I change the exact same chart to partitionLayout: PartitionLayout.sunburst, the chart renders.

@wylieconlon wylieconlon added bug Something isn't working :partition Partition/PieChart/Donut/Sunburst/Treemap chart related labels Apr 21, 2020
@wylieconlon wylieconlon changed the title Treemap infinite loop when more than one layer with valueAccessor of 0 Treemap infinite loop when more than one layer and valueAccessor of 0 Apr 21, 2020
@monfera monfera self-assigned this Apr 22, 2020
@monfera
Copy link
Contributor

monfera commented Apr 22, 2020

@wylieconlon thanks for the note. Partition accessors were designed with referentially transparent functions in mind - how does this situation arise in the code? Are you using a closure that changes the value depending on previous invocations, and is there expectation for this?

If possible, I'd like to avoid the materialization of the accessor-mapped input as the first step, as it'd be extra code, and often, performance overhead, which is why it'd be good to see if you have a need for stateful "accessors" or there can be a pure function that reproduces the issue.

@wylieconlon
Copy link
Author

@monfera I was reproducing this in Lens, where depending on the query results we sometimes get data with 0s in it. So yes, this is reproduceable without a random data set- you could instead change some of the sample objects in the storybook to include 0s.

@monfera
Copy link
Contributor

monfera commented Apr 30, 2020

Renaming this as more than one layer is not a prerequisite

@monfera monfera changed the title Treemap infinite loop when more than one layer and valueAccessor of 0 Treemap in infinite loop when valueAccessor returns zero Apr 30, 2020
monfera added a commit that referenced this issue Apr 30, 2020
Ignore not only negative values but also zero values.

fix #642
markov00 pushed a commit that referenced this issue May 4, 2020
## [19.1.2](v19.1.1...v19.1.2) (2020-05-04)

### Bug Fixes

* **partition:** elimination of zero values ([#658](#658)) ([9ee67dc](9ee67dc)), closes [#642](#642)
@markov00
Copy link
Member

markov00 commented May 4, 2020

🎉 This issue has been resolved in version 19.1.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@markov00 markov00 added the released Issue released publicly label May 4, 2020
AMoo-Miki pushed a commit to AMoo-Miki/OpenSearch-Dashboards that referenced this issue Feb 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working :partition Partition/PieChart/Donut/Sunburst/Treemap chart related released Issue released publicly
Projects
None yet
3 participants