fix(physics): stop forceAtlas2Based layout from rotating forever (#2193)#2450
Draft
bojanstef wants to merge 3 commits into
Draft
fix(physics): stop forceAtlas2Based layout from rotating forever (#2193)#2450bojanstef wants to merge 3 commits into
bojanstef wants to merge 3 commits into
Conversation
…js#2193) The FA2 repulsion scaled the pairwise force by the receiving node's degree only, making it non-reciprocal (violating Newton's third law) and injecting a net torque each tick that damping caps but never removes -> the converged layout spins forever. Remove the degree factor to restore reciprocity. Adds a deterministic regression test measuring net angular velocity about the COM.
… fix The fix changes forceAtlas2Based layouts (removing the spurious degree factor), so the 6 FA2 pinning snapshots whose graphs have edges change. Values taken from the PR CI run for environment consistency. The 2-disconnected FA2 snapshot is unchanged (degree-1 nodes are unaffected), as are all non-FA2 solver snapshots.
Author
|
@Zeophlite @Thomaash let me know what you think |
Member
|
Hi @bojanstef, first of all thanks for contributing. However, AFAIK degree based repulsion is the selling point of ForceAtlas2 (it pushes highly interconnected hubs to the periphery, making them easier to discern), removing it seems as a bad idea to me. |
Author
|
@Thomaash ah, understood. I was resolving a symptom and not the cause. Let me take another stab at it... |
…#2193) The earlier commits removed degree-based repulsion entirely to stop the perpetual rotation, but degree weighting is core to ForceAtlas2 (it pushes high-degree hubs to the periphery). The actual cause is that the degree factor was applied to only the receiving node, making the pairwise force non-reciprocal (violating Newton's third law) and injecting a net torque each tick that damping caps but never removes. Restore degree weighting in the reciprocal form from the FA2 paper (repulsion proportional to (deg(a)+1)(deg(b)+1)) by folding (degree+1) into the Barnes-Hut effective mass used both to build the tree and to evaluate the force. Hubs are still pushed to the periphery; the force is reciprocal again, so a converged layout no longer rotates. Rewrite the regression test to measure the actual best-fit rotation of node positions over a long, never-stabilizing run; the previous test measured a frozen velocity residual, which does not reflect the visible rotation. Validated across hub, barbell, scale-free and random graphs: net rotation drops 7x-5800x and the layout settles in every case. Regenerate the six affected forceAtlas2Based snapshots accordingly.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #2193. With
forceAtlas2Based, an asymmetric graph's layout converges in shape but then keeps rotating about its center of mass forever. No other solver does this.Root cause
FA2BasedRepulsionSolverscaled the pairwise repulsion by the receiving node's degree only (node.edges.length + 1). So the force on i from j is ∝deg_iwhile the force on j from i is ∝deg_j— non-reciprocal whenever the degrees differ, violating Newton's third law. That injects a small net torque every tick; velocity damping caps the resulting spin but never removes it, so the converged layout rotates at a constant terminal rate. (On a symmetric graph the per-pair torques cancel, which is why it went unnoticed.)Fix
Keep ForceAtlas2's degree-weighted repulsion — it's what pushes high-degree hubs to the periphery — but apply it symmetrically, as in the original FA2 formulation: repulsion ∝
(deg_i + 1)(deg_j + 1). This is done by folding(degree + 1)into the Barnes–Hut effective mass used both to build the tree and to evaluate the force, so the pairwise force is reciprocal again and torque-free. Hubs are still pushed outward; the rotation is gone.Tests
Rewrote
test/physics/forceatlas2-rotation.test.tsto measure the actual rotation of node positions (best-fit/Procrustes angle over a long, never-stabilizing run) rather than a velocity proxy. Deterministic (seeded positions, fixed timestep). Asserts net rotation < 5° (bug ≈ 122°, fix ≈ 0.4°); red onmaster, green with the fix.Validated across hub, barbell, scale-free and random graphs: net rotation drops 7×–5800× and the layout settles in every case, while degree-based hub spreading is preserved.
Snapshots
The fix changes converged FA2 layouts, so the 6
forceAtlas2Basedsnapshots whose graphs have edges are regenerated. The 2-disconnected FA2 snapshot is unchanged (degree-1 nodes), and no other solver's snapshots change.