Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NGL 2.2.2 doesn't show bond #1012

Open
hainm opened this issue Jan 15, 2024 · 5 comments
Open

NGL 2.2.2 doesn't show bond #1012

hainm opened this issue Jan 15, 2024 · 5 comments
Assignees

Comments

@hainm
Copy link
Contributor

hainm commented Jan 15, 2024

Hi

Please see this issue: nglviewer/nglview#1008 (comment)
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
Copy link
Collaborator

ppillot commented Jan 15, 2024

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.

@ppillot ppillot self-assigned this Jan 27, 2024
@jump2cn
Copy link

jump2cn commented Apr 16, 2024

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
Copy link
Collaborator

@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
Copy link

link89 commented Jun 6, 2024

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
Copy link

link89 commented Jun 6, 2024

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants