-
Notifications
You must be signed in to change notification settings - Fork 5
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
Correct Spheropolyhedron Class #213
Conversation
return h_cyl + h_sphere | ||
|
||
@mean_curvature.setter | ||
def mean_curvature(self, value): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the change in mean curvature require a change in both the volume of the shape and the rounding radius? And does changing both values at once result in the expected behavior?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not necessarily, no. One could set the rounding radius or change the volume of the underlying polyhedron independently in order to achieve a desired mean curvature. The challenge with that approach would be that not all possible (positive, real) values could be achieved. For example, if one wanted to adjust the curvature by changing the radius alone, then one could not achieve a curvature smaller than that of the underlying polyhedron. Presumably for this reason, the setter methods for volume and surface area for this class scale the object when setting a value. I've tried to match the behavior of the already established functions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good - Polyhedron/ConvexPolyhedron don't provide setters for mean curvature but I think that it makes sense for a rounded shape like this!
Description
There is an error in the method via which coxeter currently computes the volumes and surface area of speropolyhedra. Part of the sums of the volume & SA are based on fractions of cylinders that extend from the edges of the base polyhedron. The included angle of this cylinder is equal to the angle between the normals of the two adjoining faces, which is equal to$\pi - \phi$ , where $\phi$ is the dihedral (see e.g. equation (9) of Eric, I. M., & Michael, E. (2017). Virial Coefficients and Equations of State for Hard Polyhedron Fluids). Currently the sum is calculated only as $\phi$ . While this gives correct results for the test case of a cube ($\pi/2 = \pi - \pi/2$ ), it does not for non-orthogonal geometries.
In addition, the mean curvature of the spherobody is implemented. The (normalized) integral mean curvature is just the sum of the mean curvature of the base shape and the radius.
Motivation and Context
Types of Changes
1The change breaks (or has the potential to break) existing functionality.
Checklist: