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

Collide force doesn't handle identical initial positions well #199

Open
Nate-Wessel opened this issue Jul 15, 2021 · 7 comments · May be fixed by #214
Open

Collide force doesn't handle identical initial positions well #199

Nate-Wessel opened this issue Jul 15, 2021 · 7 comments · May be fixed by #214

Comments

@Nate-Wessel
Copy link

I'm working on a fixed force layout with collision and X and Y positioning. It's being rendered by React, and so I'm trying to optimize and speed up the force layout to reduce the expense of the simulation as my component may be resized, panned, and thus rerendered/resimulated quite often.

I found that initializing the node positions with the x and y properties greatly reduces the number of ticks needed to get the layout looking the way I want it. But it also causes some problems where the initial position is the same for several points, as in this map where I'm trying to cluster points around polygon centroids.

Screenshot 2021-07-15 at 12-28-29 Risk Demo

With another 40-50 ticks, it will settle down into something like this which is what I expect. (Possibly just due to floating point error?)

Screenshot 2021-07-15 at 12-36-36 Risk Demo

I can get the same effect with a shorter simulation by applying a deterministic (seeded) random jitter to the initial positions, but I thought it would be nice if D3 could handle this itself similar to the way the phyllotaxis arrangement around 0,0 does this for the default initial positions.

@Fil
Copy link
Member

Fil commented Jul 16, 2021

Respecting user-defined initial positions seems like the expected behavior. In fact, adding jitter to user-defined initial positions might cause problems in some cases (for example if you need all the initial x values to be exactly 0). A possibility might be to define a force that jitters positions on initialization, but I don't see how this would be easier to use than jittering the initial positions?

@Fil Fil closed this as completed Jul 16, 2021
@Nate-Wessel
Copy link
Author

I guess my suggestion is less that initial positions should be jittered (that was just my own solution and I agree that overriding users' explicit intentions would not be good) and more that the direction of the force applied by forceCollide should be. It seems to send all points outward along a common axis, rather than sending them in all directions as I would expect. I suppose there must already be some way of deciding which way to send the points in this case, but it seems to apply to all points with the shared position rather than picking a random direction separately for each.

Anyway, just a friendly suggestion. Thanks for your contributions to such an awesome library.

@Fil
Copy link
Member

Fil commented Jul 16, 2021

It should already be the case

if (x === 0) x = jiggle(random), l += x * x;

oh, I see… it seems to be because, once the two first dots have been "randomly" separated, the other coincident points are not anymore within 0 of those points, and so the force is in the direction set by the first random jitter. A bug!

@Fil Fil reopened this Jul 16, 2021
@curran
Copy link
Contributor

curran commented Jul 17, 2021

Oh wow nice catch @Fil ! That's subtle.

@Fil
Copy link
Member

Fil commented Jul 17, 2021

I have a fix here https://observablehq.com/d/dd1bcd29c2b1b850; it breaks the symmetry of the original solution for 2 coincident nodes, which is not a requirement but was kinda nice.

@Nate-Wessel
Copy link
Author

Any update on this? I just realized that it's been nearly a year. I actually have a few other use-cases now were it would be really nice to have this resolved. Happy to try and create a pull request if it helps!

Fil added a commit that referenced this issue Mar 31, 2023
@Fil Fil linked a pull request Mar 31, 2023 that will close this issue
@Fil
Copy link
Member

Fil commented Mar 31, 2023

Sorry I don't know why I had never pushed this branch from my laptop. Now PR #214.

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

Successfully merging a pull request may close this issue.

3 participants