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

Suggestion: add option for collide force to be deterministic #121

Closed
benstevens48 opened this issue Jul 12, 2018 · 6 comments · Fixed by #160 or #175
Closed

Suggestion: add option for collide force to be deterministic #121

benstevens48 opened this issue Jul 12, 2018 · 6 comments · Fixed by #160 or #175

Comments

@benstevens48
Copy link

I'd like my force layout to produce the same results given the same inputs. However, since I'm using the collide force, this is not the case because the jiggle function uses Math.random. Can I suggest having the option to pass a seed number to the collide force constructor, which can be used as a seed for a pseudo-random number generator instead of Math.random? Or possibly it could be an option which applies to the whole simulation, and then any forces that use a random number can use the same seeded pseudo-random number generator, thus always producing the same layout given the same inputs.

@alexburner
Copy link

I'd love this too, we have this cute little monster in our webpack config right now:

{
    test: /jiggle\.js$/,
    include: /node_modules\/d3-force-3d/,
    use: [
        {
            loader: 'patch.loader.js',
            options: {
                search: 'return (Math.random() - 0.5) * 1e-6;',
                replace: 'return 0.1;',
            }
        },
    ],
},

Fil added a commit that referenced this issue Jun 25, 2020
fixes #121.(maybe)

do we need to expose jiggle(seed) so people could set the seed? (I don't think so)
@Fil Fil mentioned this issue Jun 25, 2020
@Fil
Copy link
Member

Fil commented Jun 25, 2020

We could add a seeded & deterministic prng (by default, no need for it to be optional?). See #160

@Fil
Copy link
Member

Fil commented Jul 10, 2020

Please test and confirm that #160 would work for the use cases you have in mind?

@benstevens48
Copy link
Author

Probably. A couple of issues with that implementation of the random number generator though.

  1. It's not centered around 0.
  2. If a seed is provided, the result will likely be very small unless the seed is very large.

I would suggest this modification. The Math.abs is just in case the provided seed is negative.

return ((s = (a * Math.abs(seed || s) + c) % m) / m - 0.5) * 1e-6;

@Fil
Copy link
Member

Fil commented Jul 10, 2020

ah yes! thanks for catching this.

Fil added a commit that referenced this issue Jul 27, 2020
fixes #121.(maybe)

do we need to expose jiggle(seed) so people could set the seed? (I don't think so)
Fil added a commit that referenced this issue Jul 28, 2020
@mbostock
Copy link
Member

do we need to expose jiggle(seed) so people could set the seed? (I don't think so)

Yes, I think we do; otherwise the only way to reset the state of a force simulation is to reload the page. I’m thinking that the force simulation should have an assignable random source (similar to d3-random’s random.source) allowing the caller to pass in the desired random generator. If we like, we could have that default to a linear congruential generator with a fixed seed, but I’d scope that to the simulation instance, so that by creating a new simulation you can effectively reset the state of the prng. I’ll put up a PR.

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.

4 participants