nxt3AT / sankeydiagram.net

sankeydiagram.net is an easy-to-use webapp for generating Sankey Diagrams to visualize flows and budgets.
Other
75 stars 12 forks source link

Fix value for circular streams #44

Closed SimonSimCity closed 10 months ago

SimonSimCity commented 10 months ago

Thanks for providing this sankey diagram visualization! I'm actually quite happy it doesn't collapse when adding a circular stream. Just this issue: The value doesn't seem to be right...

Example: https://sankeydiagram.net/?content=PTAEGUHkFUCUGECioDaA1AggGWsgFAMYCGAdqAEYCmoRoA5APx0CUAuqACoawDiiHoPCniQskWK2YAoAO5EA5tRQN2ASxIA3SgGcALuvlT1WvQdQrQ/AGLap17agCMAVgAM7cADIACm9d2OGyc3dkQAVwAnAHtwXSiAD3i3I00dfRJ5c3Y9KIIAa1sc/IcUENBjNIMpIA===&

sankeydiagram-net-export

// SOURCE [VALUE (can be a '?')] TARGET ([COLOR])
wage [?] investing
investing [?] ETFs
ETFs [150] S&P500
ETFs [150] EuroStoxx50
investing [?] stocks
stocks [50] investing

In the current implementation, the node investing has a value of 400, whereby as I read it, it should only be 300, because the 50 are straight fed back into the node, increasing it by another 50.

I've found another library which contains a reference to an algorithm to detect circular references - maybe it helps: https://github.com/tomshanley/d3-sankey-circular

JonasDoesThings commented 10 months ago

Hmm I've looked into this issue and indeed 400 is completely wrong.

That said, I would lean to the side of 350 being to correct value of "investing". It's true that the -50 and +50 are a zero-sum game, but it's still 300 and 50 units going into the "investing" node (see my graphic).

image

What's your opinion on that @SimonSimCity?

Nonetheless I will release an update to the dev branch shortly that fixes the incorrect behavior causing 400 to be the node's calculated value. After making sure that it doesn't break any legitimate normal use, I will release it to the main branch too.

JonasDoesThings commented 10 months ago

Update: I have deployed the preliminar fix to dev, you can try it out here

SimonSimCity commented 10 months ago

You're right, 350 makes more sense 😎👍 Nice you've had time to fix it so quickly 🎉

JonasDoesThings commented 10 months ago

The fix is live on https://sankeydiagram.net/ now :)