compas icon indicating copy to clipboard operation
compas copied to clipboard

unify_cycles / face_adjaccency

Open romanarust opened this issue 1 year ago • 2 comments

Describe the bug

I have a set of vertices and faces from a correct, closed mesh, but the function unify_cycles fails. The reason for this is that when there are more than 100 faces, a different function (_face_vertices) is used for face adjacency. However, in my case, this function produces a different, and wrong result than face_adjacency, because the parameters of this function aren’t dependent on the input.

How would it be best to proceed: I could propose a fix, but should the parameters be determined based on the input (i.e determine the radius on the longest edge?), or should the user be allowed to select which function to use (unify_cycles(fast=True))? My understanding is that the _face_adjacency function was intended to increase performance. Wouldn’t it make sense for this to be user-controlled?

romanarust avatar Nov 14 '24 06:11 romanarust

perhaps both?

  • user is allowed to choose, with the default switch for over 100 faces
  • radius is None by default and in that case is computed based on some kind of heuristic
  • radius is available as a top-level parameter so it can be set by the user in case of awkward mesh geometries

tomvanmele avatar Nov 14 '24 08:11 tomvanmele

ok thank you, I'll propose something

romanarust avatar Nov 14 '24 08:11 romanarust