-
Notifications
You must be signed in to change notification settings - Fork 49
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
add biased voronoi cells feature #593
base: main
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## master #593 +/- ##
==========================================
- Coverage 55.24% 55.21% -0.03%
==========================================
Files 15 15
Lines 2576 2581 +5
Branches 38 38
==========================================
+ Hits 1423 1425 +2
- Misses 1137 1140 +3
Partials 16 16
Continue to review full report at Codecov.
|
… a point which global exists
… a point which global exists
@yuanzhou0827 Let me know if you have questions about this PR! |
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.
@yuanzhou0827 I commented on the type issues you had -- you were mixing single precision (float
, np.float32_t
and double precision (double
, np.float64_t
) values. Also it seems like master wasn't merged into this branch correctly. Let me know if you want help with that.
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.
Good progress, @yuanzhou0827! We need to add tests for this code, and expand on the documentation. Some of the tests you can add:
- Create a random system of 1000 points and box side length 10, with random radii between 0 and 1. Make sure the sum of the volumes is approximately equal to the box volume.
- Manually construct a small system (even with just 2 points) where the weights are unequal. Verify as many properties (volumes, polyhedra coordinates, weights, bonds) as you can derive.
@@ -1120,17 +1120,28 @@ cdef class Voronoi(_Compute): | |||
def __dealloc__(self): | |||
del self.thisptr | |||
|
|||
def compute(self, system): | |||
def compute(self, system, radii=None): | |||
R"""Compute Voronoi diagram. | |||
|
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.
We need to add a longer description of Voronoi diagrams and how this feature works. I think it might go best in the class docstring. It should include external links, formulas/equations, and descriptions of why someone would want to use this feature. It's important to include the several names for this feature so that it can be found via Google / documentation search. Please verify what type of weighted Voronoi diagram we are computing (additively weighted, multiplicatively weighted, or something else?), and cite whatever references you can find. You may need to refer to the voro++ documentation.
@yuanzhou0827 Have you used this feature in your research? If so, we should try to finish and merge it. If you haven't used this feature in your research then I suggest we close this pull request. |
@bdice Hey Bradley, surely i want to finish it asap. I tried some day before but have been struggling with the version conflicts. I am thinking if it is possible to close this PR and make the changes in the newest version freud? That would be easier and won't cost too much time because there are only several-lines-changes? |
@yuanzhou0827 I just did a merge commit (c1d032a) so this PR is up to date with the latest changes in freud. If you'd like to finish it, just pull those changes and you'll be up to date! I think the items needed on this PR are documentation (see this comment for some ideas) and tests (see this file for the existing Voronoi tests, just add your own tests for particles with radii in a similar style). |
cool! Thank you! i will work on the test cases and documentations in days! |
Description
Add a new possibility to have volume-polydisperse/weighted voronoi diagram.
Motivation and Context
Have weighted voronoi diagram available, thus we can do binary assembly from size-weighted voronoi cells.Resolves: #???
How Has This Been Tested?
Screenshots
Types of changes
Checklist: