Option to disable Physics #489

dremerb · 2024-05-02T16:30:30Z

I wanted the option to disable physics, because with a bigger network it takes quite some time before things settle. Also organizing is not optimal, when devices do not have coordinates yet and start flying around when another is moved.
Unfortunately, simply disabling physics puts everything at (0, 0) by default.

So I got just overengineered some static clustering.
The idea is to find a random coordinate as a center point for each rack (using the rack name as a seed yields the same "random" coordinates for every rack) and adding a small random offset by using the (unique) device name as seed.
Currently there is two magic numbers (12 in line 218 and 2 in line 225/226). They scale the resulting graph.

I recognize the idea is a bit unorthodox, but I also think that having physics enabled by default is not great for larger networks.

Note: If you consider merging this, I think the commit should be squashed, as I initially had #488 as a single branch and split it into two PRs. This makes the history pretty ugly.

dreng

You have overlooked the saved filters.

api/views.py, line 112 and below
views.py, line 713 and below.

dreng · 2024-05-03T10:40:48Z

netbox_topology_views/migrations/0009_disable_physics.py

+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('netbox_topology_views', '0007_individualoptions_group_locations_and_more'),


This is problematic, because another PR lists this 0007 as a dependency. This one is no. 0009 and should depend on 0008.

Indeed, this happened because I was working on this PR and #488 in parallel. #488 contains a migration with index 0008, therefore this got 0009 and cannot depend on 0008, since it is on another branch.

I propose leaving this comment open until either #488 or this PR gets accepted and change it accordingly as a last step.

netbox_topology_views/utils.py

dreng · 2024-05-03T10:45:28Z

netbox_topology_views/utils.py

@@ -177,7 +177,7 @@ def get_query_settings(request):
    if "show_neighbors" in request.GET:
        if request.GET["show_neighbors"] == "on" :
            show_neighbors = True
-    
+
    return filter_id, save_coords, show_unconnected, show_power, show_circuit, show_logical_connections, show_single_cable_logical_conns, show_cables, show_wireless, group_sites, group_locations, group_racks, show_neighbors


get_query_settings() has to be expanded with the new option(s).

dreng · 2024-05-03T10:49:45Z

netbox_topology_views/forms.py

+        required=False,
+        initial=False,
+        help_text=_('When enables, no forces will act on nodes in the topology and they will only move '
+                    'when dragged by hand. Devices without coordinates will be places at (0, 0) by default.')


Typo: 'placed' instead of 'places'

dreng · 2024-05-03T10:51:19Z

netbox_topology_views/api/views.py

@@ -109,7 +109,7 @@ def list(self, request):

        if request.GET:

-            filter_id, save_coords, show_unconnected, show_power, show_circuit, show_logical_connections, show_single_cable_logical_conns, show_cables, show_wireless, group_sites, group_locations, group_racks,show_neighbors = get_query_settings(request)
+            filter_id, save_coords, show_unconnected, show_power, show_circuit, show_logical_connections, show_single_cable_logical_conns, show_cables, show_wireless, group_sites, group_locations, group_racks, show_neighbors = get_query_settings(request)


You did actually nothing here except housekeeping. Please do not touch anything that does not belong to the PR. Otherwise there is more to review than necessary.

dreng · 2024-05-03T10:52:00Z

netbox_topology_views/api/views.py

@@ -109,7 +109,7 @@ def list(self, request):

        if request.GET:

-            filter_id, save_coords, show_unconnected, show_power, show_circuit, show_logical_connections, show_single_cable_logical_conns, show_cables, show_wireless, group_sites, group_locations, group_racks,show_neighbors = get_query_settings(request)
+            filter_id, save_coords, show_unconnected, show_power, show_circuit, show_logical_connections, show_single_cable_logical_conns, show_cables, show_wireless, group_sites, group_locations, group_racks, show_neighbors = get_query_settings(request)

            # Read options from saved filters as NetBox does not handle custom plugin filters
            if "filter_id" in request.GET and request.GET["filter_id"] != '':


get_query_settings() has to be expanded with the new option(s).

Fixed, Github cannot figure it out because reasons.

dreng · 2024-05-03T10:58:59Z

netbox_topology_views/views.py

+        # make seed unique for devices for spread
+        if hasattr(device, "rack"):
+            random.seed(device.name)
+        # spread devices around the rack center coordinates


Randomization will probably lead to a different layout when exporting to XML. Have you tested that?

Yes, where previously all coordinates were (0, 0), they now are set to some values.
The diff between the export of "physics enabled" and "physics disabled" consists of all mxGeometry tags. When save_coordinates is enabled, the diff before and after moving a single node (and the same enabled/disabled setting for physics) is only one mxGeometry tag.

Re-rendering with disabled physics (hence with "random coordinates") yields the same file every time.

netbox_topology_views/forms.py

dreng · 2024-05-03T11:32:39Z

Hi,

first of all, thank you for your contribution.

Which accepted FR do you close with this PR actually? If it's #436, that one is under review for a good reason. It is absolutely not clear how to implement the FR. You've made a solution that only fits your personals needs. We should always find solutions that fits everyones needs or discuss until we get to a good compromise.

dremerb · 2024-05-06T08:27:05Z

netbox_topology_views/views.py

+    else:
+        # draw devices clustered by their rack neighbours
+        # for center coordinates of a rack, use the rack name as seed (if available)
+        random.seed(device.rack.name if hasattr(device, "rack") else device.name)


Suggested change

random.seed(device.rack.name if hasattr(device, "rack") else device.name)

random.seed(device.rack.name if hasattr(device, "rack") else device.site)

site is a mandatory field when creating a new Device. Also would remove the if in line 222, simply setting the new seed to device-name for all Devices.

dremerb · 2024-05-06T08:42:52Z

This PR does not have a FR, I just wanted the option to disable physics, since in my graph it was a chore to organize the nodes when things were moving constantly. Of course that changes once you turn on "save coordinates", but that still requires every node to be selected/moved once.

#436 and #122 both touch the idea of disabling physics, but it is not their main focus as far as I understand.

You've made a solution that only fits your personals needs. We should always find solutions that fits everyones needs or discuss until we get to a good compromise.

Agree with the second part. I built this as a simple fix to everything being located at (0, 0) when I disables physics in the module by setting the key to False for all nodes and edges. Also it fits my personal needs, but since I am working with a decently large setup, I hope it holds some value that can be applied generally.
The proposed code is a "best effort" clustering, instead of running a full ground state simulation on the server. While it does not look as pretty as the result of vis.js' physics simulation, it is obtained a lot faster and does not have a round shape (which is unusual for topology graphs).
A (in my opinion sane) assumption I made was, that users will re-organize the graph manually anyway, therefore I prioritized clustering racks over it looking pretty.

I maybe should have made it clearer, but this more or less is a Feature Request that offers an idea how to solve the issue of not having coordinates when simply disabling physics in vis.js.
If it is easier to discuss first, I am open to close this, create an Issue to discuss the option to disable physics.

mr1716 · 2024-05-13T10:31:44Z

This would be a great change to do!

dremerb · 2024-05-21T08:48:01Z

Messed up managing my branches and pushed a merge that does not belong here. Removed those commits with a force push.

mattieserver · 2024-07-09T07:39:55Z

I tried this out but i am not convinced the seeding works very well (at least for my own testing).
I am not sure this is the way we want to implement it, however it gave me and idea on what to try.

I will not merge this for now.

dreng requested changes May 3, 2024

View reviewed changes

netbox_topology_views/forms.py Show resolved Hide resolved

dremerb commented May 6, 2024

View reviewed changes

dremerb force-pushed the disable_physics branch from 8c95afb to 5a0d8e3 Compare May 21, 2024 08:46

Daniel Bremer added 19 commits June 6, 2024 10:16

Add option to disable gravity and smooth cables

cf8a01c

fix variable initialization

e403814

disable physics on nodes as well

2e65a7c

set better names

6caf782

Add options to form

eca7ea3

add fields to model

28eabfa

use settings values instead of config file

1abe84e

moved options to settings

2cb5c28

add new migrations

f7b2265

default locations grouped by rack

3440950

better clustering

0286c58

break away smooth cables

e9b125e

pretty comments

9698597

was broken, now it's not

0df9d12

fix naming schema

40e4cf3

fix var not being renamed

601421f

changes from review

1e50def

add missing filter

ab5a7f4

optimize coordinate generation precedence

f3b9985

dremerb force-pushed the disable_physics branch from 5a0d8e3 to f3b9985 Compare June 6, 2024 08:20

mattieserver added the status: under review Further discussion is needed to determine this issue's scope and/or implementation label Jul 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Option to disable Physics #489

Option to disable Physics #489

dremerb commented May 2, 2024

dreng left a comment

dreng May 3, 2024

dremerb May 7, 2024

dreng May 3, 2024

dreng May 3, 2024

dreng May 3, 2024

dreng May 3, 2024

dremerb May 7, 2024

dreng May 3, 2024

dremerb May 7, 2024

dreng commented May 3, 2024

dremerb May 6, 2024

dremerb commented May 6, 2024

mr1716 commented May 13, 2024

dremerb commented May 21, 2024

mattieserver commented Jul 9, 2024

	random.seed(device.rack.name if hasattr(device, "rack") else device.name)
	random.seed(device.rack.name if hasattr(device, "rack") else device.site)

Option to disable Physics #489

Are you sure you want to change the base?

Option to disable Physics #489

Conversation

dremerb commented May 2, 2024

dreng left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dreng commented May 3, 2024

Choose a reason for hiding this comment

dremerb commented May 6, 2024

mr1716 commented May 13, 2024

dremerb commented May 21, 2024

mattieserver commented Jul 9, 2024