session.models.assign_id() failure

[Tristan Croll]

Clipper's Symmetry_Manager class is designed to take over control of an AtomicStructure (taking over its ID if it's a top-level model) and some set of Volume subclasses:

class Symmetry_Manager(Model):
    '''
    Handles crystallographic symmetry and maps for an atomic model.
    '''
    def __init__(self, model, mtzfile=None, map_oversampling=1.5,
        min_voxel_size = 0.5, spotlight_mode = True, spotlight_radius=12,
        hydrogens='polar', ignore_model_symmetry=False,
        set_lighting_to_simple=True, debug=False):

        # A bunch of code irrelevant to the problem.

        id = model.id
        self.add([model])
        session.models.add([self])
        if len(id) == 1:
            session.models.assign_id(self, id)

The task of associating a map with a model is exposed to the command line as clipper associate {VolumesArg} toModel {StructureArg}.

... which seems to work fine at first:

open 6eyd
open 3983 from emdb
clipper assoc #2 to #1

# Success

but (in a fresh session)

open 6eyd
open 3983 from emdb
close #1
open 6eyd
clipper assoc #2 to #1

cxclipper associate #2 toModel #1Traceback (most recent call last):
  File "/opt/UCSF/ChimeraX-daily/lib/python3.6/site-packages/chimerax/cmd_line/tool.py", line 229, in execute
    cmd.run(cmd_text)
  File "/opt/UCSF/ChimeraX-daily/lib/python3.6/site-packages/chimerax/core/commands/cli.py", line 2586, in run
    result = ci.function(session, **kw_args)
  File "/home/tic20/.local/share/ChimeraX/0.8/site-packages/chimerax/clipper/cmd.py", line 52, in associate_volumes
    mgr = get_map_mgr(to_model, create=True)
  File "/home/tic20/.local/share/ChimeraX/0.8/site-packages/chimerax/clipper/symmetry.py", line 149, in get_map_mgr
    sh = get_symmetry_handler(structure, create=create)
  File "/home/tic20/.local/share/ChimeraX/0.8/site-packages/chimerax/clipper/symmetry.py", line 145, in get_symmetry_handler
    return Symmetry_Manager(structure)
  File "/home/tic20/.local/share/ChimeraX/0.8/site-packages/chimerax/clipper/symmetry.py", line 431, in __init__
    session.models.assign_id(self, id)
  File "/opt/UCSF/ChimeraX-daily/lib/python3.6/site-packages/chimerax/core/models.py", line 474, in assign_id
    del mt[model.id]
KeyError: (1,)

KeyError: (1,)

File "/opt/UCSF/ChimeraX-daily/lib/python3.6/site-packages/chimerax/core/models.py", line 474, in assign_id
del mt[model.id]

See log for complete Python traceback.

Traceback (most recent call last):
  File "/opt/UCSF/ChimeraX-daily/lib/python3.6/site-packages/chimerax/core/triggerset.py", line 130, in invoke
    return self._func(self._name, data)
  File "/opt/UCSF/ChimeraX-daily/lib/python3.6/site-packages/chimerax/model_panel/tool.py", line 127, in <lambda>
    lambda *args, ft=self._fill_tree: ft(always_rebuild=True))
  File "/opt/UCSF/ChimeraX-daily/lib/python3.6/site-packages/chimerax/model_panel/tool.py", line 160, in _fill_tree
    parent = item_stack[0] if len(item_stack) == 1 else item_stack[len_id-1]
IndexError: list index out of range

Error processing trigger "frame drawn": list index out of range:
IndexError: list index out of range

File "/opt/UCSF/ChimeraX-daily/lib/python3.6/site-packages/chimerax/model_panel/tool.py", line 160, in _fill_tree
parent = item_stack[0] if len(item_stack) == 1 else item_stack[len_id-1]

If in models.assign_id() I make the following substitution:

<        del mt[model.id]
>        mt.pop(model.id, None)

... then it works again - but then why does it work in the first instance above?

[Eric Pettersen]

Tom wrote the model.assign_id routine, so I _think_ this is his..

[Tom Goddard]

This is a bug in clipper/symmetry.py line 428

self.add([model])

where the code adds an already opened model (model = 6eyd structure) to self = Symmetry_Manager where the Symmetry_Manager has not yet been opened. Adding to an open model to an unopen model is an error. This could be fixed by first removing the opened model before adding it to the unopened model

session.models.remove([model])
self.add([model])

or you could add the new Symmetry_Manager model and then add model to it (adding an open model to another open model is handled).

The model management code could be smarter and see if you are trying to do that and remove the model from the open models but currently it does not do that.

I don't see why opening, closing and reopening 6eyd has this problem while not closing avoids it. But the current code is wrong and expected to cause havoc with the model ids.

[tic20@…]

... huh. I could have sworn that used to be legal. Never mind - easy fix.

 
 
Tristan Croll
Research Fellow
Cambridge Institute for Medical Research
University of Cambridge CB2 0XY
 

 

[Tristan Croll]

Adjusting my code does take care of it - but the way I was doing it is in keeping with the advice in #599. Happy to run with the new approach if that is now the standard.

[goddard@…]

Yep my comment in #599 was wrong.  Adding an open model to an unopen model is not supported.  I should probably make it check and close the model in that case.  I'm not really keen on automatically handling these weird cases because it is likely to lead to more confusion because the model close will cause trigger fires and it will probably not be obvious when you look at code m1.add(m2) why this caused a model close.  So I would say the best way to do this that won't have unexpected consequences is to open Model m1, and then add m2 (which is already open) to m1.  Then there are no implicit or explicit model closes that see surprising when you are just reparenting a model.

[Tristan Croll]

Just doing some Xmas tidying up... this has all been working fine for quite some time.