ComputeAdjacencyList Assignment Open3D #7094
Conversation
adityaranigaon
commented
Dec 12, 2024
adityaranigaon
commented
- Implement C++ function
- Add gtests
- Add pybind
- Add pytests
|
Thanks for submitting this pull request! The maintainers of this repository would appreciate if you could update the CHANGELOG.md based on your changes. |
There was a problem hiding this comment.
Thanks @adityaranigaon , this looks good. Some thoughts:
- Can we change the name to
ComputeAdjacencyMatrix() / compute_adjacency_matrix(). The adjacency list uses an equivalent but slightly different format. We have that as part of the Eigen API, so having the same name may confuse users. - Do add these as triangle attributes as well. They are needed for other algorithms to be added later (e.g. mesh filtering).
|
|
||
| triangle_mesh.def("compute_adjacency_list", &TriangleMesh::ComputeAdjacencyList, | ||
| "Return Mesh Adjacency Matrix in CSR format using Triangle indices attribute."); | ||
|
|
There was a problem hiding this comment.
Describe returned pair explicitly for users that don't know CSR format.
There was a problem hiding this comment.
I am guessing that adjst() would be added to vertex attribute and adjv() to triangle attributes ?
However the check HasVertexAttribute will fail because it checks the length of attribute is equal to the number of vertices, however as per CSR format the length of adjv is "num_vertices + 1"
There was a problem hiding this comment.
Good point. We need a different container for storing attributes for the TriangleMesh as a whole. I've added a new container to TriangleMesh for this purpose. Can you use that? I'll update the python bindings / tests for the new container later.
| EXPECT_TRUE( adjv.GetLength() > 0); | ||
| EXPECT_TRUE( adjst.GetLength() > 0); | ||
| EXPECT_EQ(adjst.ToFlatVector<int64_t>()[adjst.GetLength()-1], adjv.GetLength()); | ||
| } |
There was a problem hiding this comment.
You can also add a test for Euler characteristic (n_faces + n_vertices - 2 = n_edges).
face = triangle, n_edges = length of csr_col.
| [4, 7, 9], | ||
| [5, 6, 10], | ||
| [5, 6, 11] | ||
| ] |
There was a problem hiding this comment.
You can get this from mesh.triangle.indices.numpy() May need to be lexicographically sorted. (np.lexsort)
| } | ||
|
|
||
| core::Tensor tris_cpu = | ||
| GetTriangleIndices().To(core::Device()).Contiguous(); |
There was a problem hiding this comment.
TODO: Also enable on cuda and sycl.
| std::unordered_map< size_t, std::set<size_t> > adjacencyList; | ||
| auto insertEdge = [&adjacencyList](size_t s, size_t t){ | ||
| adjacencyList[s].insert(t); | ||
| }; |
There was a problem hiding this comment.
Edges are undirected. For a triangle mesh, 2 neighboring faces will share an edge. If they are oriented in-consistently, the directions will be opposite, i.e. if one face is [0, 1, 2], another may be [2, 1, 3]. In this case we will get duplicate entries in the adjacency list. Essentially, we just need the lower triangle of the adjacency matrix - the upper triangle is redundant.
There was a problem hiding this comment.
Also, some tri meshes are not manifold (degenerate) - this can lead to situations where an edge is shared by 3 faces. Using a set as the value type does address this.
PS: Why do we need an unordered_map<size_t, set> instead of a simpler vector<set>?
There was a problem hiding this comment.
yeah, vector would work. I will update
To store adjacent_vertex and adjacent_index_start keys to represent adjacency matrix.
