Solid Liquid is 'broken' in 0.5.2
Investigate
Comments (7)
-
reporter -
reporter Find which version still works and which doesn't so that we can do a diff
-
reporter @xiyudu @samnola I'm starting to take a look at this (this will serve as the issue for all the solid liquid problems). I've currently identified the issue with getClusterSizes:
- The C++ function is returning a std::vector but the cython code is expecting something more akin to the pointer like in the rest of freud
simple enough to fix, but we need to know/identify what it should be returning...the documentation doesn't make sense (says returns largest cluster size, but that's clearly not the case, or it should be a separate function...) Anyway, that's a trivially-solvable problem (and even has this marked as a todo in the documentation, so yay?)
Second problem...that it's broken and/or slow. This isn't as trivial since I don't have any test code to run on. One/both of you need to either:
- Get me a minimal test case:
- all required data
- script that I can run with <code>$: bash test.sh</code> or <code>$: python test.py</code>
- Profile using a profiler yourself (I recommend the Xcode profiler included in /Applications/Xcode/Contents/Applications/Instruments) which will tell you which parts of the code that it takes the most time on, that way I can start to figure out what's happening
Let me know how you want to proceed. I don't have a ton of time to tackle this, but I want to work through it and get it working.
-
@xiyudu Do you know if this code is still broken?
-
@bdice I think it is fixed by now
-
There is a unit test file test_order_SolLiq.py and the tests in there appear to be passing. @xiyudu is that enough to make you feel comfortable that it's working now? Or is there some additional validation that we should do?
-
- changed status to closed
Closing since Chrisy reported that it is fixed, and we have unit tests.
- Log in to comment
Find a starting point