search

GEMDAT-repos / GEMDAT

Python toolkit for molecular dynamics analysis
https://gemdat.readthedocs.io
Apache License 2.0
22 stars 3 forks source link

`test_volume_to_structure` fails intermittently #138

Closed v1kko closed 1 year ago

v1kko commented 1 year ago

Its about a 50/50 chance to pass, we should probably investigate what is causing it: https://github.com/GEMDAT-repos/GEMDAT/actions/runs/6169747931/job/16744628799#step:7:75

stefsmeets commented 1 year ago

Specifically, test_volume_to_structure is failing.

  File "/opt/hostedtoolcache/Python/3.10.13/x64/lib/python3.10/site-packages/scipy/spatial/distance.py", line 2375 in squareform
  File "/opt/hostedtoolcache/Python/3.10.13/x64/lib/python3.10/site-packages/pymatgen/core/structure.py", line 4149 in merge_sites
  File "/home/runner/work/GEMDAT/GEMDAT/src/gemdat/volume.py", line 222 in to_structure
  File "/home/runner/work/GEMDAT/GEMDAT/tests/integration/trajectory_test.py", line 34 in test_volume_to_structure
stefsmeets commented 1 year ago

Also happens locally and on 3.11.

  File "/home/stef/python/gemdat/src/gemdat/segmentation.py", line 35 in watershed_pbc
  File "/home/stef/python/gemdat/src/gemdat/volume.py", line 163 in _peaks_to_props
  File "/home/stef/python/gemdat/src/gemdat/volume.py", line 194 in to_structure
  File "/home/stef/python/gemdat/tests/integration/trajectory_test.py", line 34 in test_volume_to_structure
stefsmeets commented 1 year ago

I marked the test as pytest.mark.skip in #139 until we find a solution.

stefsmeets commented 1 year ago

I'm going to revert the volume_to_structure implementation to use the watershed work-around with padded arrays. Our current implementation watershed_pbc is (1) not correct and (2) crashes.

I like the idea of having a watershed algorithm that considers periodic boundary conditions, but this would require either patching scikit-image or writing our own implementation that correctly wraps around the edges. This takes a bit of time to develop.

Todo