Add dense_longlong to all integer dtype parameters by KarlLeell · Pull Request #15 · libigl/libigl-python-bindings

KarlLeell · 2019-07-26T23:25:29Z

No description provided.

…-python-bindings

fwilliams

There are a lot of commented out checks. Are these causing tests to fail? I added some other notes here and there.

fwilliams · 2019-08-05T18:46:54Z

src/ears.cpp


 npe_begin_code()

+  //assert_valid_3d_tri_mesh_faces(f, "f");


Why is this commented out?

There are some tests commented out, basically because they are either from the unmerged PR, like assert_valid_tet_or_tri_mesh_faces, or they are not implemented yet, just I think they should be there so I put it, like assert_valid_3d_tri_mesh_faces.

fwilliams · 2019-08-05T18:47:14Z

src/edges.cpp

 npe_arg(f, dense_int, dense_long, dense_longlong)
 npe_begin_code()

+  //assert_valid_tet_or_tri_mesh_faces(f, "f");


Why is this commented out?

fwilliams · 2019-08-05T18:47:24Z

src/euler_characteristic.cpp


 npe_begin_code()

+  //assert_valid_tet_or_tri_mesh_faces(f, "f");


Why is this commented_out?

fwilliams · 2019-08-05T18:49:14Z

src/exact_geodesic.cpp

+  //assert_cols_equals(vs, 1, "vs");
+  //assert_cols_equals(fs, 1, "fs");
+  //assert_cols_equals(vt, 1, "vt");
+  //assert_cols_equals(ft, 1, "ft");


Why are these commented out?

Also, I'm not sure if these will always have shape (N, 1). They might have shape (1, N). It might make sense to have something like assert_is_vector(vs, "vs") which checks both conditions.

True, these ones I'm not sure (I'm not even sure if they can be omitted), so I added them but comment them out.

fwilliams · 2019-08-05T18:49:54Z

src/harmonic.cpp

 #include <igl/harmonic.h>

-//TODO: is is only dense_int because min_quad_with_fixed_data hardcodes int, fixme
+//TODO: is is only dense_int because min_quad_with_fixed_data hardcodes int, FIXME


Is this comment still true?

It seems min_quad_with_fixed has been templated now, I will try to fix this later

teseoch · 2019-08-07T19:04:45Z

src/barycenter.cpp

 npe_begin_code()
-  assert_valid_3d_tri_mesh(v, f);
+
+  assert_valid_tet_or_tri_mesh(v, f);


it is by simplex

you are indeed correct

teseoch · 2019-08-07T19:05:06Z

src/bfs_orient.cpp

+npe_arg(f, dense_int, dense_long, dense_longlong)
 npe_begin_code()

+  //assert_valid_tet_or_tri_mesh_faces(f, "f");


why commented?

teseoch · 2019-08-07T19:05:35Z

src/boundary_facets.cpp

+npe_arg(t, dense_int, dense_long, dense_longlong)
 npe_begin_code()

+  // TODO: need an option for multiple shapes


can you elaborate?

Here t can be either #t by 3 or #t by 4, and currently I think we cannot check this

assert_valid_tet_or_tri_mesh_faces

teseoch · 2019-08-07T19:05:47Z

src/boundary_loop.cpp

+npe_arg(f, dense_int, dense_long, dense_longlong)
 npe_begin_code()

+  //assert_valid_tet_or_tri_mesh_faces(f, "f");


teseoch · 2019-08-07T19:06:40Z

src/circumradius.cpp


 npe_begin_code()

+  assert_valid_tet_or_tri_mesh(v, f);


if it is by 3 it is tri_mesh3d not tets

teseoch · 2019-08-07T19:11:21Z

src/euler_characteristic.cpp


 npe_begin_code()

+  assert_valid_tet_or_tri_mesh(v, f);


i think only tri

teseoch · 2019-08-07T19:11:41Z

src/face_components.cpp

-npe_arg(f, dense_int, dense_long)
+npe_arg(f, dense_int, dense_long, dense_longlong)
 npe_begin_code()
+    //assert_valid_3d_tri_mesh_faces(f, "f");


teseoch · 2019-08-07T19:12:52Z

src/internal_angles.cpp


 npe_begin_code()

+  assert_valid_tet_or_tri_mesh(v, f);


teseoch · 2019-08-07T19:13:01Z

src/is_edge_manifold.cpp


 npe_begin_code()

+  //assert_valid_tet_or_tri_mesh_faces(f, "f");


teseoch · 2019-08-07T19:13:29Z

src/is_irregular_vertex.cpp

 npe_arg(f, dense_int, dense_long)

 npe_begin_code()
+  assert_valid_tet_or_tri_mesh(v, f);


correct but misleading :D

KarlLeell added 7 commits July 24, 2019 20:14

add dense_longlong to all integer parameters

d7e2829

Merge branch 'master' of https://github.com/geometryprocessing/libigl…

d03fecf

…-python-bindings

Merge branch 'master' of https://github.com/geometryprocessing/libigl…

9fbd7b8

…-python-bindings

Merge branch 'master' of https://github.com/geometryprocessing/libigl…

f0906e9

…-python-bindings

adding shape checks

7527389

add shape checks to all src files

f37a316

fixing tests input

9525b3d

fwilliams reviewed Aug 5, 2019

View reviewed changes

KarlLeell and others added 4 commits August 5, 2019 15:14

Fixed input data shapes

31f0224

fix shape checks

bb7efa3

Merge branch 'master' into master

3addcec

change data path

1e76609

teseoch reviewed Aug 7, 2019

View reviewed changes

KarlLeell merged commit 1e76609 into libigl:master Aug 8, 2019

stigersh mentioned this pull request Apr 27, 2023

openmp and per_face_normals pybind11 wrapper crash #193

Closed


		npe_begin_code()

		//assert_valid_tet_or_tri_mesh_faces(f, "f");

Conversation

KarlLeell commented Jul 26, 2019

Uh oh!

fwilliams left a comment

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants