CellNetwork.cell_neighbors() modification #1449

2twenity · 2025-03-24T10:43:35Z

What type of change is this?

Bug fix in a backwards-compatible manner.
New feature in a backwards-compatible manner.

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

I added a line to the CHANGELOG.md file in the Unreleased section under the most fitting heading (e.g. Added, Changed, Removed).
I ran all tests on my computer and it's all green (i.e. invoke test).
I ran lint on my computer and there are no errors (i.e. invoke lint).
I added new functions/classes and made them available on a second-level import, e.g. compas.datastructures.Mesh.
I have added tests that prove my fix is effective or that my feature works.
I have added necessary documentation (if appropriate)

Some description.

CellNetwork.cell_neighbors

I've been trying to analyze connections between building elements using CellNetwork.cell_neighbors method and found out that simple common faces search could not work, because faces can point at the vertices with same coordinates, but which have different indexes, because there's no duplicate vertex check in CellNetwork.add_vertex method. Therefore I've modified the comparison logic to simple common vertices search. If 2 cells have more than 2 vertices in common, they might be adjacent. (Maybe a condition that points don't lie on the same line should be considered as well). This method performed quite well in a graph construction process of an HVAC received from Speckle.

I've added this particular CellNetwork to tests/compas/datastructures/fixtures folder.

CellNetwork.add_mesh

Also I've implemeted add_mesh method to CellNetwork which simple decomposes mesh to vertices and faces, checks if it's manifold and adds as a Cell to CellNetwork

… added

…mented

tomvanmele · 2025-03-28T16:40:10Z

hi,

thanks for this.
would you mind fixing the lint in the code so the workflows can run properly?

we will automate this in the future, but for now this needs to be done locally...

thanks!

tomvanmele

i (think i) understand what you're trying to do, but (if i understood correctly) i think we should do it differently.

with what you propose, the cell network will not care about the welding of vertices in some cases while it does care in others. this will lead to very confusing results.

we should perhaps rather change the behaviour wrt welding, and/or include a setting wrt this behaviour on a global level...

tomvanmele · 2025-03-29T07:27:52Z

src/compas/datastructures/cell_network/cell_network.py

+    def add_mesh(self, input_mesh: Mesh):
+        if input_mesh.is_manifold():
+            # mesh.unify_cycles()
+            unpacked_mesh = input_mesh.to_vertices_and_faces()


vertices, faces = ...

tomvanmele · 2025-03-29T07:28:23Z

src/compas/datastructures/cell_network/cell_network.py

+            if self._max_vertex == -1:
+                vertex_diff = 0
+            else:
+                vertex_diff = self._max_vertex + 1


this can be simplified to

vertex_offset = self._max_vertex + 1

tomvanmele · 2025-03-29T07:29:12Z

src/compas/datastructures/cell_network/cell_network.py

+                vertex_diff = self._max_vertex + 1
+
+            for vertex in unpacked_mesh[0]:
+                self.add_vertex(attr_dict = {"x": vertex[0], "y": vertex[1], "z":vertex[2]})


self.add_vertex(x=vertex[0], y=vertex[1], z=vertex[2])

tomvanmele · 2025-03-29T07:31:27Z

src/compas/datastructures/cell_network/cell_network.py

@@ -728,6 +728,32 @@ def add_cell(self, faces, ckey=None, attr_dict=None, **kwattr):

        return ckey

+
+    def add_mesh(self, input_mesh: Mesh):


i have the felling this already exists but under a different name, no @romanarust ?

no, this does not yet exist

tomvanmele · 2025-03-29T07:32:02Z

src/compas/datastructures/cell_network/cell_network.py

@@ -4033,7 +4059,7 @@ def cell_face_neighbors(self, cell, face):
        return nbrs

    def cell_neighbors(self, cell):
-        """Find the neighbors of a given cell.
+        """Find the neighbors of a given cell based on common vertices.


i think this should be a different function because it does something else than what was intended by the original one...

Actually I was even thinking about proposing one more datastructure like Mesh Network. Something more practice oriented, because that's not common to have a building represented as masses or adjacent rooms only. Moreover some parts in a model can have clashes, or be adjacent on a face to face level only, or can have common vertices.
So briefly the initial functionality:

analyzing meshes adjacency (including collision etc)

storing some metadata on a mesh level (attr dict but for meshes)

mesh network to graph conversion (each node is a mesh centroid, edge could have some metadata about type of adjacency like collision or face adjacency)

graph algorithms

integration with compas viewer

Basically all of these could be added to CellNetwork as well, if it has no clashes with initial idea of a CellNetwork

what you describe is already under development here
https://github.com/BlockResearchGroup/compas_model

i think this should be a different function because it does something else than what was intended by the original one...

Agree. Additionally, heavy processing tasks like clash detection shouldn't be tied to the CellNetwork, as they fall under mesh processing. Identifying nearby meshes for clash detection (which I assume is the goal of this function) would be better handled with something like KDtree for spatial awareness.

The goal of this function is to convert meshes to a topologic spatial graph. Clashes were just a thing to consider, the general assumption is that the model is relatively clean with required openings etc. But now I see that maybe CellNetwork should remain more lightweight, especially considering the compas_model developments. I'll have a look at compas_model and investigate how it could be used for this task

romanarust · 2025-03-29T14:52:14Z

src/compas/datastructures/cell_network/cell_network.py

+
+            for face in unpacked_mesh[1]:
+                new_face = [vertex + vertex_diff for vertex in face]
+                self.add_face(new_face)


fkey = self.add_face(new_face)

romanarust · 2025-03-29T14:52:33Z

src/compas/datastructures/cell_network/cell_network.py

+            for face in unpacked_mesh[1]:
+                new_face = [vertex + vertex_diff for vertex in face]
+                self.add_face(new_face)
+                faces_lst.append(self._max_face)


faces_lst.append(fkey)

romanarust · 2025-03-29T15:01:21Z

Hi @2twenity! Thank you for the PR!

2twenity

Thanks for the review! I've modified add_mesh function and reverted back cell_neighbors method, because I find it more appropriate for compas_model. Method itself should be further developed as well. Also I've added add_mesh test

2twenity added 16 commits March 19, 2025 16:48

First version of cell neighbors search

78aa12d

Debug things were added to the function

699c5b5

Debug things were added to the function

15d4f03

Some bug fixes

c87a0c2

Final condition of polygons intersection added. Cell neighbors pytest…

ce730cd

… added

Polygon bug was fixed

41c5627

Function was simplified into more bruteforce algorithm

22a2170

The mesh adjacency function was simplified to common vertices check

fb087fd

Function that adds a manifold mesh as a cell to CellNetwork was imple…

fe82e1d

…mented

Cell neighbors function fixed

760c5c8

Cell Neighbors function bug fixed

697665d

Cell Neighbor function bug fixes

da4b248

Cell Neighbors function test added

e816686

Cell Neighbors test modified

b45e7ca

Cell Neighbours test commented out

5e86329

Some adjustments

b3f2f0e

tomvanmele requested a review from romanarust March 28, 2025 16:37

tomvanmele assigned tomvanmele and romanarust Mar 28, 2025

tomvanmele reviewed Mar 29, 2025

View reviewed changes

romanarust reviewed Mar 29, 2025

View reviewed changes

Reviewd changes applied

7bf234c

2twenity commented Apr 6, 2025

View reviewed changes

2twenity added 2 commits April 6, 2025 15:40

Merge branch 'compas-dev:main' into main

246a7c1

Merge branch 'compas-dev:main' into main

bfea5c9

		@@ -728,6 +728,32 @@ def add_cell(self, faces, ckey=None, attr_dict=None, **kwattr):

		return ckey


		def add_mesh(self, input_mesh: Mesh):

CellNetwork.cell_neighbors() modification #1449

Are you sure you want to change the base?

CellNetwork.cell_neighbors() modification #1449

Uh oh!

Conversation

2twenity commented Mar 24, 2025

What type of change is this?

Checklist

Some description.

CellNetwork.cell_neighbors

CellNetwork.add_mesh

Uh oh!

tomvanmele commented Mar 28, 2025

Uh oh!

tomvanmele left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

romanarust commented Mar 29, 2025

Uh oh!

2twenity left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tomvanmele left a comment •

edited

Loading