openmc-dev / openmc

OpenMC Monte Carlo Code
https://docs.openmc.org
Other
699 stars 444 forks source link

Polygon order and line intersection #2959

Open FusionSandwich opened 2 weeks ago

FusionSandwich commented 2 weeks ago

Micah has already submitted a few submissions for the polygon issue. Here's my submission.

Description

Currently, there is an issue with the polygon function in that the points need to be listed clockwise otherwise "ValueError: Polygon cannot be self-intersecting". Should add in a function to automatically originate into the correct configuration.

Alternatives

Or add documentation to specifically state the order in the points that need to be in

Example

Test case points = [ (-16.4,20), # Start at one corner (-11.4,20), # Go to the next corner in a clockwise or counterclockwise direction (3.5,7.5), # Continue in the same direction (-2.0,7.5) # Return to the starting corner ] test = openmc.model.Polygon(points, basis='xy') Same points but in a different order provides error points = [ (-11.4,20), (-16.4,20), # Start at one corner # Go to the next corner in a clockwise or counterclockwise direction (3.5,7.5), # Continue in the same direction (-2.0,7.5) # Return to the starting corner ] test = openmc.model.Polygon(points, basis='xy')

Compatibility

No. Uses base OpenMC

eepeterson commented 2 weeks ago

Hi @FusionSandwich, thanks for raising the issue. The problem here is that the input points need to be ordered by the user. They can be ordered either clockwise or counter-clockwise, it doesn't matter, but the edges between neighboring points in the list (including the implicit edge from the final point back to the first) cannot intersect with each other. This is what happens in your second test case when you reorder the points. For a general complex polygon there is no one "correct" way to order the points so there is no way for the code to correct it for the user.

FusionSandwich commented 2 weeks ago

This might be a more clear example of the problem"points = [ (-16.4,20), (-2.0,7.5), (-11.4,20),
(3.5,7.5)
] test = openmc.model.Polygon(points, basis='xy')". This also provides a self-intersecting error. The points are put in a counter-clockwise . This is the error that I and others made trying to get the polygon working. So I think it'd be worthwhile to at least clarify the documentation on the open website.

FusionSandwich commented 2 weeks ago

Overall I think it might not be the most intuitive function. An example on the website and a description of how the point placement is read would be welcomed I think. " An Nx2 array of points defining the vertices of the polygon." Make it seem like it doesn't matter the choice of points grouped together.