missing tethers, mysterious floating sidechains look like ligands

[Elaine Meng]

It is not acceptable that when somebody is showing ribbons and then displays some residues, the sidechains are completely detached. Open any structure that has strands, show some residues in those strands. Example:

open 3bp5
hide
delete ~protein
contacts /a&protein test /b&protein reveal true

I'm told hiding and reshowing the ribbon is the workaround, but people are not going to figure this out, and it's not a good behavior to have in our 0.9 release.

[Conrad Huang]

I think this is one of the ramifications of the #1486 change.

[Elaine Meng]

Forget that example, I think it has two bugs in it (one from "contacts"). Here is a simpler one:

open 3bp5
show :trp
view /b:195

[Greg Couch]

One option is to back out Tristan's change. Or convert to cython to speed things up. Conrad's the ribbon code expert.

[Tristan Croll]

I think backing out that change entirely isn't the right answer. The bug that it was supposed to fix becomes serious for any model with lots of separate chain fragments - in the original example I used (5wsg, where m.polymers(missing_structure_treatment=m.PMS_TRACE_CONNECTS)) returns 173 fragments) the code was generating about 1,800 more tethers than necessary.

If I revert the change Greg made in #1486 to my original patch, then Elaine's examples both work correctly (although as an aside I noticed that the 'contacts' example triggers the generation of chain trace pseudobonds between adjacent displayed CAs despite the ribbon being displayed - not sure why).

So the question is, why does my patch break nucleotide tethers? Will see if I can find out today (I really can't afford the performance hit to ISOLDE that reverting it would invoke).

[Tristan Croll]

OK, I have some answers on this.

What the code does:

• for each iteration over a particular chain fragment, for each residue in that chain, for each atom name in _RibbonPositions, it adds matching atoms and their corresponding anchor point on the spline as keys and values, respectively, to the self._ribbon_spline_backbone dict.

• _ribbon_spline_backbone is not cleared from one chain fragment to the next. Therefore, prior to my patch every loop over a new chain fragment would add a new tether for every matching atom encountered up to that point - every N, CA, C, P, O5', C5', C4', C3' and O3' for all fragments - hence the steep geometric blow-out for large and complex models. The aim of my patch was to restrict it to only the atoms designated as "tether atoms" for that particular fragment (the atoms returned by Residues.get_polymer_spline()). For protein residues get_polymer_spline() returns the CAs, so everything works as expected - but for nucleic acids it returns C5', which isn't so useful - not only is C5' hidden, but the ribbon spline passes almost exactly through it anyway. The original code was still drawing correct-looking tethers because it was creating them for *everything*.

Anyway, one simple quick fix is to simply clear the _ribbon_spline_backbone dict at each iteration once it's no longer needed:

<            mask = tether_atoms.visibles
<            tether_atoms = tether_atoms[mask]
<            spline_coords = spline_coords[mask]

>            self._ribbon_spline_backbone = {}

That gets rid of *most* of the problem, but it still leaves unnecessary tethers being created for the hidden backbone atoms in _RibbonPositions. To deal with that would require a separate explicit definition of exactly which atoms should have tethers drawn. Something along the lines of:

        for rlist, ptype in polymers:
            # Always call get_polymer_spline to make sure hide bits are
            # properly set when ribbons are completely undisplayed
            any_display, atoms, coords, guides = rlist.get_polymer_spline()
>           from numpy import in1d
>           tethered_atoms = atoms[in1d(atoms.names, ("CA", "C4'", "C3'"))]

[Tristan Croll]

Sorry, make that last:

            any_display, atoms, coords, guides = rlist.get_polymer_spline()
            if not any_display:
                continue
            residues = atoms.residues
>           from numpy import in1d
>           tethered_atoms = residues.atoms[in1d(residues.atoms.names, ("CA", "C4'", "C3'"))]

[Tristan Croll]

... or probably a better option:

    def _ribbon_spline_position(self, ribbon, residues):
>       from chimerax.atomic import Atom
        for n, r in enumerate(residues):
            first = (r == residues[0])
            last = (r == residues[-1])
            for atom_name, position in self._RibbonPositions.items():
                a = r.find_atom(atom_name)
<               if a is None or not a.is_backbone():
>               if a is None or not a.is_backbone() or a.hide&Atom.HIDE_RIBBON:
                    continue

[Greg Couch]

The intent of computing the tether positions is to cache where they are so if a bond (and eventually a pseudobond) to a backbone atom is shown, then that atom will be shown with a tether to it. The "unnecessary" tether atoms are actually necessary because those bonds might not be shown when the ribbon is computed. The original patch worked for amino acids because it only computed tethers for the ribbon guide atoms and CA is a guide atom. And typically, the only additional bond is to the CA (except for proline). But there are H-bonds to the O and N, so those should be shown with tethers if the H-bonds are displayed.

If the quick fix of clearing self._ribbon_spline_backbone is sufficient, then let's go with that.

[Elaine Meng]

The H-bonds to backbone thing is a separate issue that has always been that way and is well known (at least to me), apparently independent of the newly missing tethers and CA-CB bonds.  I would only consider the latter as bugs.  This separate issue of the detached H-bonds is discussed in the documentation for “hbonds reveal”:

http://rbvi.ucsf.edu/chimerax/docs/user/commands/hbonds.html#reveal

"Floating pseudobonds in the presence of cartoon can be hidden with the command 'cartoon suppress false', with the side effect of showing any backbone atoms that are displayed but suppressed by the cartoon."

[Greg Couch]

The floating H-bond to backbone bug is a separate but related bug. If the tether positions are not computed for hidden backbone atoms, then that bug can't be fixed.

[Elaine Meng]

Nobody wants to show a bunch of extra balls and tethers when they are in the ribbon abstraction — it would be a hideous mess. With ribbons only, the pseudobonds themselves are still useful to see H-bonding along helix, or quick identification of beta-bulge in a sheet. The actual fix is to project the pseudobond endpoints onto the ribbon,  ticket #183, as in Chimera.

[Greg Couch]

That projection will need the computed tether positions, so that doesn't alter my point about the "unnecessary" tether locations are actually necessary.

Anyway, the quick fix is in.

[Greg Couch]

Turns out this fix breaks Atom.ribbon_coord, so the nucleotide tubes no longer connect to the ribbon.

[Greg Couch]

Explicitly include Tristan in discussion.

An alternate solution would be to add a flag that limits the tether computation that can be turned on and off. That was ISOLDE and turn it on while it is updating the coordinates, and turn it off before the last frame so subsequent modifications by the user will appear.

[Greg Couch]

Tristan, we're backing out the tether optimization since it breaks other parts of ChimeraX. Do you have a test case that we could use for investigating possible solutions?

[Tristan Croll]

Anything large with lots of chain fragments will show up the issue. 5wsg is a good example, and has the added benefit of containing both protein and nucleic acid. As per the original bug report in #1486, the one-liner:

sum([len(t[3]) for t in m._ribbon_tether if t[3] is not None])

reports the number of ribbon tether positions (2437552 tethers for 9879 residues at the time, 3478631 with the current code). If I understand properly now the correct number for this model should be no more than 40216 (four for each protein residue, six for each nucleic acid residue).

If you'll bear with me, I have one more attempt at a fix:

            # self._ribbon_spline_backbone = {}
            from numpy import in1d
            ratoms = atoms.unique_residues.atoms
            mask = in1d(ratoms.names, list(self._RibbonPositions.keys()))
            indices = tether_atoms.indices(ratoms)
            tether_atoms = tether_atoms[indices]
            spline_coords = spline_coords[indices]

This passes the 2bna and 6cmn tests in #1981, and generates 31522 tether positions for 5wsg. I've tried it against 6cmn with the following combinations:

• nucleotides as tube/slab, cartoon created before displaying atoms
• nucleotides as tube/slab, cartoon created after displaying atoms
• nucleotides as atoms, cartoon created before displaying atoms
• nucleotides as atoms, cartoon created after displaying atoms

... and if I turn off suppressBackbone, the tethers for the previously hidden atoms are all there too.

[Greg Couch]

So your fix limits the part of _ribbon_spline_backbone used to the current set of residues. I've made a different fix. I changed _ribbon_spline_position(ribbon, residues) to return the positions that will need to be added to _ribbon_spline_backbone. And after tether_atoms is computed for those positions, update _ribbon_spline_backbone with those positions. And the patch becomes:

             # Cache position of backbone atoms on ribbon
             # and get list of tethered atoms
-            self._ribbon_spline_position(ribbon, residues)
+            positions = self._ribbon_spline_position(ribbon, residues)
             from numpy.linalg import norm
             from .molarray import Atoms
-            tether_atoms = Atoms(list(self._ribbon_spline_backbone.keys()))
-            spline_coords = array(list(self._ribbon_spline_backbone.values()))
+            tether_atoms = Atoms(list(positions.keys()))
+            spline_coords = array(list(positions.values()))
+            self._ribbon_spline_backbone.update(positions)
             if len(spline_coords) == 0:
                 spline_coords = spline_coords.reshape((0,3))

Testing with 5wsg, there are 31522 tethers. And the draw time goes down from 25.7 to 11.2 seconds.

[Elaine Meng]

Are these changes going into the build, and if so, can you clarify as to whether it’s in 0.9 or development?  Thanks.

[Greg Couch]

The fix is in both 0.9 and the daily build.

[Elaine Meng]

Looks good in today’s builds, both daily development and release candidate: in my test case, tethers are present.  Also, the alpha-carbons are visible (#1966) and the nucleotide tubes are not detached from ribbon (#1981).

[Greg Couch]

Hi Tristan, thank you for all of your help with this. Sorry it took so long for me to understand all of the important parts of your patch. Does the current patch meet your needs?

[tic20@…]

Sorry - was buried in other tasks all day today. Will test properly tomorrow.
 

[Tristan Croll]

Tested and working perfectly, thanks!

[Greg Couch]

Done!