search

qcscine / molassembler

Chemoinformatics toolkit with support for inorganic molecules
https://scine.ethz.ch/download/molassembler
BSD 3-Clause "New" or "Revised" License
31 stars 7 forks source link

Graph.Cycles gives incorrect output #13

Closed ViktoriiaBaib closed 9 months ago

ViktoriiaBaib commented 11 months ago

Hello!

I noticed that scine_molassembler.Cycles gives incorrect atomic positions when I iterate through cycles - it just repeats the same numbers. Screen Shot 2023-09-21 at 2 50 24 PM

When ligands are different, the issue persists with atomic positions being mixed from different cycles into one.

Could you look into it?

My expected output is: [[(0, 3), (0, 4), (1, 3), (1, 4)], [(0, 7), (0, 8), (5, 7), (5, 8)], [(0, 11), (0, 12), (9, 11), (9, 12)]] bino3.xyz.zip

jan-grimo commented 11 months ago

This is a bug in the python bindings. The Cycles iterator returns references to a vector which gets updated in every iteration. It seems pybind doesn't deep-copy the lists it retrieves in every iteration, which leads to the last relevant cycle to be repeated across the list of edges.

It'll be fixed in our GitLab instance for the next release, but in the meantime this patch should fix it for you. Can you confirm that this works for the other cases you have, too?

weymutht commented 9 months ago

The patch mentioned above is included in Molassembler 2.0.1, released today.