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

Rethink automatic starting of simulations? #45

Closed
urbanhop opened this issue Jul 17, 2016 · 10 comments · Fixed by #165
Closed

Rethink automatic starting of simulations? #45

urbanhop opened this issue Jul 17, 2016 · 10 comments · Fixed by #165
Assignees

Comments

@urbanhop
Copy link

Reading the Force-Directed Graph example, I find it hard to understand, when the simulation starts.

According to the the API, "The simulator starts automatically". So that would be here:

var simulation = d3.forceSimulation()
    .force("link", d3.forceLink().id(function(d) { return d.id; }))
    .force("charge", d3.forceManyBody())
    .force("center", d3.forceCenter());

Hmmm, without nodes, there wont be much to simulate. Continue reading the code, I assume, it might happen here:

simulation
  .nodes(graph.nodes)
  .on("tick", ticked);

Now it has nodes and can work. But it still has no links. Hmmm. So does it run considering the other forces? Well, Javascript is singlethreaded, so the time callback cannot execute anyway. So presumably it the timer fires, but the callback will be run after this function, or more exactly, after the XmlHttpRequest callback is handled. Lots of pondering. Continue reading

simulation.force("link")
  .links(graph.links);

I assume that now the simulation has all arguments and can run considering the link forces. Which in fact is correct. Anyway, I find the API a bit confusing in this point (otherwise the new force API is absolutely ingenious!).

Debugging reveals, that force.initialize() is called 3 times, computing

nodeById = map$1(nodes, id),

twice, unnecessarily. So maybe it makes sense to have an API method alike

simulation.start(nodes, links)

or more abstract (or precise)

simulation.start(all parameters required to run the configured simulation)

? For me that would have been clearer. Most functions would fall into category configure while start, restart, stop would be category run.

thx so much for d3
urbanhop

@mbostock
Copy link
Member

mbostock commented Jul 17, 2016

It starts upon creation. That is what is meant by “automatically”. If you prefer you can explicitly stop the force layout upon creation and then start it again later. (However stopping the force layout does not prevent the forces from being reinitialized when the nodes change, since starting and stopping only affects the internal timer, and the simulation can be run manually by calling simulation.tick.)

You can change the nodes after the force layout starts. Yes, this does cause the constituent forces for be reinitialized, but this cost is typically negligible so it doesn’t seem urgently necessary to optimize this. I don’t want to debounce this work until the next tick because it makes debugging slightly harder, but I suppose it would be an option.

You could not implement the type of simulation.start API you propose without the simulation knowing about all possible forces and their parameters. The design is intended to be generic and extensible in that the simulation can be run with arbitrary forces that have arbitrary behavior and arbitrary parameters; the simulation does not know what links are, for instance. Furthermore forces can be bound and unbound dynamically, so it doesn’t make sense to pass all the parameters through start. I suppose you could use weakly (string) named parameters defined in the simulation as is done with the forces, but that seems worse than having the forces define their own parameters and by worry about potential name conflicts.

There is a slight problem with the example on that the simulation can run for a tick with an empty graph (no nodes). This is mostly harmless especially since the ticked listener is not yet registered, but it does mean that the simulation can cool slightly while the data is loading. I can see if there is an easy way to avoid this, though I don’t think it will require any changes to the API. (Deferring creation of the simulation until the data loads, for example, would do it.)

@urbanhop
Copy link
Author

Think its fine as it is. After some hours working with it and now reading your reply, I got the feeling for the API, which favors convenience over minor inefficiencies.

Thank you for your time.

@mbostock mbostock self-assigned this Jul 17, 2016
@mbostock
Copy link
Member

I am reopening this issue as a reminder to myself to fix the issue you raised with the example. Thank you for your feedback!

@mbostock mbostock reopened this Jul 17, 2016
@urbanhop
Copy link
Author

urbanhop commented Jul 17, 2016

I ended up with this sequence:

var canvas = document.querySelector("canvas"),
    context = canvas.getContext("2d"),
    width = canvas.width,
    height = canvas.height;

d3.json("miserables.json", function (error, graph) {
    if (error) throw error;

    var simulation = d3.forceSimulation()
        .nodes(graph.nodes)
        .force("link",
            d3.forceLink()
            .id(d => d.id)
            .links(graph.links))
        .force("charge", d3.forceManyBody())
        .force("center", d3.forceCenter(width / 2, height / 2))
        .on("tick", ticked);

    function ticked() { ...

which is simple, but limited to cases where reheating and restarting is not required. (any maybe has even more issues I am not aware of)

@mbostock mbostock changed the title Simulation API cosmetics - Suggestion to make it more clear when the simulation starts (or is started). Example of initializing force simulation should wait until data is loaded. Sep 8, 2016
@mbostock
Copy link
Member

mbostock commented Mar 3, 2017

Some options:

  1. Defer automatic starting until the first tick listener is scheduled. This way, a simulation won’t start until someone is listening. This would be backwards-compatible with the current behavior, avoids needing simulation.stop when computing a static layout, and reduces the likelihood of the simulation starting before it‘s ready. (I say likelihood because you can still register your tick listener before configuring the simulation… This would fix the example, however.)

  2. Disable automatic starting, period. This would not be backwards-compatible with d3@4, but it’s how d3@3 did it.

  3. Defer automatic starting until nodes is non-empty. This feels a little too magical because a simulation could have forces that do things even if the simulation has no nodes.

  4. Just fix the example to call simulation.stop upon creation, and then simulation.start when the data is loaded. Or don’t define the simulation until the data is loaded.

@mbostock mbostock changed the title Example of initializing force simulation should wait until data is loaded. Rethink automatic starting of simulations? Mar 3, 2017
@IPWright83
Copy link

@mbostock on option 1, wouldn't that cause a problem if you opt for a static layout with no progressive rendering? I'd typically run the force and subscribe to end to render the positioned output, rather than on each tick.

@rohit0710

This comment has been minimized.

@Fil
Copy link
Member

Fil commented Jun 25, 2020

I've implemented solution 1 in #165. We start when any listener is added (ie "tick" or "end") so it should cover @IPWright83 's use case too (though I'd be reassured if it could be tested independently).

demo notebook @ https://observablehq.com/d/0e3586acc3d9cd55

@IPWright83
Copy link

@Fil really disappointingly my new job doesn't give me the chance to use D3 anymore, which is such a shame as it's an amazing library.

I will see if I can find some time to test though

Fil added a commit that referenced this issue Jul 27, 2020
@Fil Fil mentioned this issue Jul 27, 2020
5 tasks
Fil added a commit that referenced this issue Jul 28, 2020
@Fil Fil mentioned this issue Jul 28, 2020
4 tasks
@Fil
Copy link
Member

Fil commented Sep 1, 2020

We finally decided against this change. Use .stop() then .restart() when the data is ready.

@Fil Fil closed this as completed Sep 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
5 participants