search

nglviewer / ngl

WebGL protein viewer
http://nglviewer.org/ngl/
MIT License
664 stars 169 forks source link

NGL 2.2.2 doesn't show bond #1012

Open hainm opened 8 months ago

hainm commented 8 months ago

Hi

Please see this issue: https://github.com/nglviewer/nglview/issues/1008#issuecomment-1868855451 I am able to reproduce with NGL 2.2.2 too

image

File is here (need to change .txt to .cif): https://github.com/nglviewer/nglview/files/13765118/last_molecule_r_0-pos-1.txt

ppillot commented 8 months ago

The core cif file parsing functions expects the field atom_site_type_symbol to properly infer the element. This cif file does not have it: it only contains the atom_site_label. It seems that, in this case, we could default to infer element based on the later, in case atom_site_type_symbol is missing.

jump2cn commented 5 months ago

hi team, I noted that the browser console warns with 'more than 500 atoms...', it's come from this code.

https://github.com/nglviewer/ngl/blob/master/src/structure/structure-utils.ts#L606-L607

it will work well for me when I change the check limit to 99999. can we have a bigger count limit or the count limit can be a configured parameter?

fredludlow commented 5 months ago

@jump2cn - That code is inferring bonds between atoms in the same residue, and the 500 is there to avoid an explosion in complexity - I guess it's kind of an arbitrary number but 99999 seems a little high - your file has ~1500 atoms? We could set it to 2500.

I was trying to find an example where it would behave more poorly - it's doing a nearest neighbour search using a kdtree then for each pair looking up to see if they would reasonably be bonded. In your case where there's no disorder that nearest neighbour search isn't going to be a problem even if every atom is queries, most will return only a few neighbours. For something like an NMR structure with 20 models of a large ligand all overlaid I guess it could get expensive, but I couldn't find an example quickly where it was a problem.

link89 commented 3 months ago

How about just make this threshold configurable and still use 500 as the default value, so that the user can decide the right value by theirselve.

link89 commented 3 months ago

Or can I specific bonds in the data file before feeding it to nglviewer, so that it's up to user to decide instead of having nglviewer to guess by distance?