Skip to content
This repository was archived by the owner on Jul 29, 2019. It is now read-only.

check for mass value of zero on vis.js Network nodes. #3133

Closed
pontusvision opened this issue Jun 5, 2017 · 4 comments
Closed

check for mass value of zero on vis.js Network nodes. #3133

pontusvision opened this issue Jun 5, 2017 · 4 comments

Comments

@pontusvision
Copy link

I was updating an old library to the latest vis.js network, when I spotted quite an odd behaviour, where only one node out of hundreds would appear. After a fair amount of debugging, I noticed that I was accidentally setting the mass some of the nodes to zero. This was causing havoc, with a NaN eventually as the x and y coords.

I've created the following jsfiddle to demonstrate it:
http://jsfiddle.net/1y8L119L/

Setting the mass to 1 or higher solves the issue; however, the code used to work with a mass of 0 back in version 4.3.0.

Please add the extra safety check to prevent the pain I have just had to go through on other distracted silly programmers that set the mass to zero.

@wimrijnders
Copy link
Contributor

There's two years between 4.3.0 and current 4.20.0. I honestly tried to dig back through time to see what changed, but I can't manage it. So I'll skip trying to clarify what changed in the meantime to make this happen.

But really I have to agree with you. The value zero should be tested in some way and reported. I'll be paranoid and have a look if negative values should be disqualified as well. Thanks for reporting.


Background info:

Then, as now, barnesHut was the default physics solver, which in the docs comes with the warning for the mass field:

Values lower than 1 are not recommended.

Methinks that zero qualifies.

@wimrijnders
Copy link
Contributor

It turns out that setting negative mass is a really bad idea. The nodes make a sincere attempt at escaping and disappear into the distance, never to stop. I'm inclined to block negative mass as well.

@wimrijnders
Copy link
Contributor

wimrijnders commented Jun 6, 2017

The question now is how to handle this. Should vis.js just plain throw an error when this happens? Or perhaps a warning and then reset the masses (to 1 would be most obvious)?

Update: I'm going for warning and reset to 1.

wimrijnders added a commit to wimrijnders/vis that referenced this issue Jun 6, 2017
Fix for almende#3133

Option-field 'node.mass` must be >= 0.
Checks have been added at the nodes level, for both nodes-global and nodes specific options.
In addition, an internal check has been added for `NodeHandler.defaultOptions`.

The documentation has been adjusted for this change.
@wimrijnders
Copy link
Contributor

I've added the checks, they should be in the next release. Thanks for reporting!

Please close the issue if you're happy with this.

yotamberk pushed a commit that referenced this issue Jun 13, 2017
* Protect Network from zero and negative mass values

Fix for #3133

Option-field 'node.mass` must be >= 0.
Checks have been added at the nodes level, for both nodes-global and nodes specific options.
In addition, an internal check has been added for `NodeHandler.defaultOptions`.

The documentation has been adjusted for this change.

* Fix whitespace
primozs pushed a commit to primozs/vis that referenced this issue Jan 3, 2019
* Protect Network from zero and negative mass values

Fix for almende#3133

Option-field 'node.mass` must be >= 0.
Checks have been added at the nodes level, for both nodes-global and nodes specific options.
In addition, an internal check has been added for `NodeHandler.defaultOptions`.

The documentation has been adjusted for this change.

* Fix whitespace
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants