Skip to content

Distance#310

Open
janbridley wants to merge 22 commits intomainfrom
distance
Open

Distance#310
janbridley wants to merge 22 commits intomainfrom
distance

Conversation

@janbridley
Copy link
Copy Markdown
Collaborator

Description

Motivation and Context

Types of Changes

  • Documentation update
  • Bug fix
  • New feature
  • Breaking change1

1The change breaks (or has the potential to break) existing functionality.

Checklist:

Comment thread coxeter/shapes/_distance2d.py Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Yes, I think this is right (and better too, there could have been issues with the old code if someone initialized a polygon with the vertices out of order). Plus, tests are still passing

Comment thread tests/test_spheropolyhedron.py Outdated
)

np.testing.assert_allclose(distances, true_distances)
np.testing.assert_allclose(displacements[1:7], true_displacements)
Copy link
Copy Markdown
Collaborator Author

@janbridley janbridley May 1, 2026

Choose a reason for hiding this comment

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

@rynoliphant do you have the other displacement values written out?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The reason the other displacement values weren't written out was because there are multiple possible displacements to the surface (besides point [4,3,3], which I think actually has only one valid displacement). Point [3,3,3] is in the center of the spheropolyhedron and has 6 valid displacements (one to each face), point [4,4,4] sits on a vertex of the inner cube and has infinite valid displacements, point [4,4,3] sits on an edge of the inner cube and has infinite valid displacements, and point [4,3,3] sits on a face of the inner cube and has 1 valid displacement (which I can add, it should be [radius, 0,0] )

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Noted -- what does the internal code do in this case? Is it left up to whichever is closer in floating point?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants