Skip to content

Update to round_corner and round_corners Region methods #502

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

JackB-Ansys
Copy link
Collaborator

@JackB-Ansys JackB-Ansys commented Apr 25, 2025

Previously, you had to specify a valid radius for corner rounding, or an exception was raised.

Old behaviour:

  • If the radius is too large for the corner (due to the length of the adjacent entities) we raise an Exception ""Corner radius is too large for these entities. You must specify a smaller radius."

New behaviour:

  • If the radius is too large for the corner (due to the length of the adjacent entities) we try to find the maximum possible radius that is suitable for the corner and the adjacent entities. If a new radius cannot be found in 100 iterations, raise the original error.

This update means that the Adaptive Templates corner rounding function behaviour is more in line with the in-built Motor-CAD Standard Template geometry corner rounding.

Also add _consolidate_lines Region method. This checks a region entities and consolidates multiple line entities into a single line entity where possible:

  • Without "consolidation"
    image

  • With "consolidation"
    image

It is useful to consolidate lines, otherwise the corner rounding functionality is limited by the lengths of the shorter lines.

…date_lines Region method. Add _round_corner Region method.
@github-actions github-actions bot added the enhancement New features or code improvements label Apr 25, 2025
@@ -836,11 +871,11 @@ def round_corner(self, corner_coordinate, radius):
if converged == False:
raise Exception("Cannot find intersection. Check if radius is too large")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this exception, it might be worth using one of the more specific exceptions (probably ValueError, see https://docs.python.org/3/library/exceptions.html). This would mean that in the try/except block later, we can check specifically for a ValueError, rather than a bare except.

# the lengths of the adjacent entities.
for index in range(len(adj_entity_lengths)):
j = adj_entity_lengths[index]
if j < 2 * distance:
raise Exception(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above, maybe use ValueError instead of Exception?

try:
# try to round the corner with the specified radius
self._round_corner(corner_coordinate, radius, adj_entity_lengths)
except Exception as e:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we use ValueError instead of Exception in _round_corner() then we can instead use
except ValueError as e: instead of except Exception as e:
This would mean we would not inadvertently try to handle some unrelated exception that could be raised.

corner_coordinate, new_corner_radius, adj_entity_lengths
)
break
except Exception as e:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again this could be ValueError instead of Exception

Whether to maximise the possible radius if the radius provided is too large.
"""
# consolidate Line entities in the region where possible
self._consolidate_lines()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we always want to do this, or should it be optional? In your example picture
image
We might want to allow rounding to include entity 3, but not entity 2, so our corner rounding does not collide with the magnet.

try:
# try to round the corner with the specified radius
self._round_corner(corner, radius, adj_entity_lengths)
except Exception as e:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above, use ValueError instead of Exception?

# try to round the corner with the new shorter radius
self._round_corner(corner, new_corner_radius, adj_entity_lengths)
break
except Exception as e:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above, use ValueError instead of Exception?

for index in range(len(adj_entities)):
j = adj_entities[index]
if j.length < distance:
# check that the distances by which the adjacent entities are shortened are less than half
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this being over conservative? I can think of some cases where you'd expect the original line to be shortened to less than half its original length:
image

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New features or code improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants