stur86 / magresview-2

MagresView 2.0 - NMR crystallography visualisation app
https://stur86.github.io/magresview-2/
MIT License
2 stars 5 forks source link

molecule detection for 1D chains (polymers) #20

Open jryates opened 2 years ago

jryates commented 2 years ago

When selecting a molecule in the attached cif file I don't get the complete molecule. It works for other structures I have tried. I'm not sure what the issue is - but I haven't thought about it too hard (renamed to txt, as GitHub doesn't accept .cif files) magres_mol_error.txt .

jkshenton commented 2 years ago

Thanks -- good to get tough cases! I had a look at this example in vesta and it looks like infinite connected chains of atoms -- what should the correct 'select molecule' behaviour be in such cases?

If I load your example in MGV2 with 'Load as molecular crystal' ticked, the molecule selector seems to work a little better at least...

dch0ph commented 2 years ago

Yes, it looks like a polymer. Mercury [the CCDC visualisation tool] shows curious ... for two of the C-C bonds: image

The ... are presumably at the edges of the unit cell.

Special code for polymeric structures is quite low on our priority list, although this may relate to the difficult for working with framework inorganics in MV - these are kind of polymers.

jryates commented 2 years ago

Thanks - I should have remembered what this cif file actually was! Yes, it is a polymer. I've changed the name of the issue to reflect this. I think it is worth keeping it open as a low-priority issue. It might be quite simple to include 1D periodic boundary conditions in the molecule finder. But there is the wider question of what should the molecule selector do if it fails to find a sensible molecule - report its best guess, give a warning etc.

dch0ph commented 2 years ago

Yes, I think this is an important general point. I feel that a lot of CCP-NC code is quite minimal in terms of warnings etc. - everything is fine for normal cases, but there are rarely checks for things going wrong.

If there isn't one already (suspect not), I think it would be useful to have a mechanism for warnings / information, whether a pop-up box with OK. A red warning banner (with a close box) at the bottom.

stur86 commented 2 years ago

I'm not going to maintain this any more personally since I am leaving, but would like to point out that issues like this are more appropriately raised with https://github.com/stur86/crystvis-js, as that is what takes care of molecule detection and all other low level functionality like handling periodicity currently. MagresView only handles the higher levels, namely, the visualisation of NMR data on top of the structure and user interaction.