Remove duplicate vertices in Prism object

[oskooi]

Closes #1520.

[ahoenselaar]

Wouldn't one want to only consider consecutive vertices?

[stevengj]

Also, shouldn't this happen in libctl in the low-level constructor?

[oskooi]

Wouldn't one want to only consider consecutive vertices?

This is true in the case of a GDS file for which the shapes must be weakly simple polygons. This means that the polygon has segments which can intersect but not cross. This also means that representing holes requires connecting their boundary to the boundary of the enclosing shape which can give rise to duplicate vertices that are not consecutive.

In the general definition of a prism which involves convexity, any duplicate vertex would need to be removed whether it is consecutive or not.

Also, shouldn't this happen in libctl in the low-level constructor?

Sure, we can move this feature to libctl and then add a test in Meep.

[ianwilliamson]

This is true in the case of a GDS file for which the shapes must be weakly simple polygons. This means that the polygon has segments which can intersect but not cross. This also means that representing holes requires connecting their boundary to the boundary of the enclosing shape which can give rise to duplicate vertices that are not consecutive.

In the general definition of a prism which involves convexity, any duplicate vertex would need to be removed whether it is consecutive or not.

Does the issue described in #1520 actually extend to the case of having any duplicated vertices? I only showed the segfault with the duplicated first / last vertex while having subpixel smoothing enabled. Though, I could also see how there would be issues with having consecutive vertices.

Support for GDS-style polygons in the prism strikes me as being particularly useful given #1501, but if you do want to move forward with this approach, I would suggest raising an error instead of silently removing vertices. This would be quite confusing for users.

[stevengj]

To me, a polygon with duplicate consecutive vertices seems like a well-defined limiting case, that seems like a polygon with one fewer vertex. (e.g. the limit of a quadrilateral as a side length goes to zero should be a triangle, not an error)

[stevengj]

e.g. matlab polyshape automatically deletes duplicate and even co-linear vertices. https://www.mathworks.com/matlabcentral/answers/606836-why-does-the-polyshape-function-deletes-the-vertices-in-the-input-when-there-are-no-duplicated-poi … and it seems pretty common in geometry packages to support deleting consecutive duplicate vertices, even if they are only approximately equal.