cif file saved from model with children causes segmentation fault on load

[Tristan Croll]

Two-parter here:

1. If I have an AtomicStructure with child models underneath it, there doesn't seem to be (or at least I can't find) a keyword in the command-line save to save only the top-level model. That is, if I have models 1.1, 1.1.1, 1.1.2 etc., then

save file.cif #1.1

saves all of them to a single .cif file. Attempting to open said .cif file causes a segmentation fault:

Fatal Python error: Segmentation fault

Current thread 0x00007f3f81c8e740 (most recent call first):
  File "/home/tic20/apps/chimerax/lib/python3.6/site-packages/chimerax/core/atomic/mmcif.py", line 45 in open_mmcif
  File "/home/tic20/apps/chimerax/lib/python3.6/site-packages/chimerax/core/io.py", line 352 in open_data
  File "/home/tic20/apps/chimerax/lib/python3.6/site-packages/chimerax/core/io.py", line 392 in open_multiple_data
  File "/home/tic20/apps/chimerax/lib/python3.6/site-packages/chimerax/core/models.py", line 348 in open
  File "/home/tic20/apps/chimerax/lib/python3.6/site-packages/chimerax/core/commands/open.py", line 57 in handle_unknown_kw
  File "/home/tic20/apps/chimerax/lib/python3.6/site-packages/chimerax/core/commands/open.py", line 104 in open
  File "/home/tic20/apps/chimerax/lib/python3.6/site-packages/chimerax/core/commands/cli.py", line 2366 in run
  File "/home/tic20/apps/chimerax/lib/python3.6/site-packages/chimerax/cmd_line/tool.py", line 171 in execute
  File "/home/tic20/apps/chimerax/lib/python3.6/site-packages/chimerax/cmd_line/tool.py", line 64 in keyPressEvent
  File "/home/tic20/apps/chimerax/lib/python3.6/site-packages/chimerax/core/ui/gui.py", line 167 in event_loop
  File "/home/tic20/apps/chimerax/lib/python3.6/site-packages/ChimeraX_main.py", line 572 in init
  File "/home/tic20/apps/chimerax/lib/python3.6/site-packages/ChimeraX_main.py", line 663 in <module>
  File "/home/tic20/apps/chimerax/lib/python3.6/runpy.py", line 85 in _run_code
  File "/home/tic20/apps/chimerax/lib/python3.6/runpy.py", line 193 in _run_module_as_main
/home/tic20/bin/ChimeraX: line 7:  1628 Segmentation fault      (core dumped) $CHIMERA_HOME/bin/ChimeraX $*

Is there a keyword (or could one be added) to specify saving only the model described by the atomspec?

[Tom Goddard]

Our current thinking about hierarchy of models is that parent models should only be grouping models, basically models that don't draw anything and don't contain data. If you instead make parent models that contain data and draw graphics you will find that almost every user command is only able to operate on the whole tree of models. To remedy this we would need a syntax for specifying only a specific model and not including its children. No one has proposed such a syntax. Alternatively every command could have options saying not to include child models, although this probably is not workable because by the time a model specifier (like #1) is parsed, the command code simply gets a lists of models and does not know if the user specified "#1 childrenOnly true" or "#1.1-3 childrenOnly true".

Since a mechanism for commands to act only on parent nodes we suggest only using parent nodes for grouping.

From Python code you can save just a single parent atomic model as an mmCIF file.

[Tom Goddard]

Please attach the mmCIF pile that causes a crash on open. Reassigning to Greg to debug that.

[tic20@…]

Done. Sorry it's a little on the large side.

On 2017-05-31 18:58, ChimeraX wrote:

test.cif​

[tic20@…]

Added by email2trac

[tic20@…]

OK, I can live with that (with a little rearranging). I think this would 
be made easier if models loaded via the 'open' command were always 
automatically placed inside a top-level grouping model, to provide a 
ready-made container for anything the user wants to add. Otherwise, if I 
want to add models associated with an existing one I run into a choice:
  - transplant the existing model to a new position under a suitable 
parent and add the new models to the same parent, or
  - start polluting the top-level space with new models that aren't 
obviously associated with the original.

The first option is easy enough to do once you know how (I use it when 
creating a clipper CrystalStructure from an existing AtomicStructure - 
the AtomicStructure becomes a child of the CrystalStructure object), but 
it isn't currently an official method as far as I know (I use the code 
pasted below). I could see the second starting to become quite painful 
in some big bioinformatics situations.


def move_model(session, model, new_parent):
     '''
     Picks up a model from the ChimeraX model tree and transplants
     it (with all its children intact) as the child of a different model.
     '''
     model._auto_style = False
     mlist = model.all_models()
     model_id = model.id
     if new_parent in mlist:
         raise RuntimeError('Target model cannot be one of the models 
being moved!')
     for m in mlist:
         m.removed_from_session(session)
         mid = m.id
         if mid is not None:
             del session.models._models[mid]
             m.id = None
     session.triggers.activate_trigger('remove models', mlist)
     if len(model_id) == 1:
         parent = session.models.drawing
     else:
         parent = session.models._models[model_id[:-1]]
     parent.remove_drawing(model, delete=False)
     parent._next_unused_id = None
     new_parent.add([model])


On 2017-05-31 18:57, ChimeraX wrote:

[goddard@…]

The most common use case for ChimeraX is open one model and do not later try to group it with any other models.  Your proposal makes the user interface substantially worse for this most most common use, giving two levels in the model hierarchy when there is no need.

If one open command produces multiple models (e.g. opening a PDB ensemble, a map time series, or integrated hybrid model), then a group is automatically made that contains those.

If you open one model, and later want to open another model and group it with the first, then it makes most sense that you make the group when it becomes apparent that you need the group, and you reparent the original model, as you are doing.

[tic20@…]

Ok, but then would it be possible to provide a built-in version of this method? It's something that I'm sure would be used in other contexts.

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

 

[Eric Pettersen]

Maybe the open command could have a “group” keyword that controls whether the model(s) are grouped, and if not specified you get the default behavior of grouping multiple models and not grouping single models…

—Eric

[Tom Goddard]

I don't think an open command "group" option would get much use -- users don't think ahead, nor do they like to memorize exotic options to get the job done. So it is preferable that code that know it wants to make a group make the group itself.

If you want something like a move_model() routine put into the distribution I can do that. But I don't understand why the code you provided is removing the model from the session and running triggers itself. It should be much simpler than that. The "rename" command can change the id number of a model and create new parent group models if needed. You might want to look at

core/commands/rename.py

Basically it just removes the model from the models list and adds it with a new id, just a few lines of code.

[tic20@…]

Reading through core/commands/rename.py, I see this:

def _reparent_model(session, m, id, p):
     mids = _new_model_ids(m, id, p)
     ml = session.models
     ml.remove([m])
     for model, new_id, parent in mids:
         model.id = new_id
         ml.add([model], parent = parent)


It's late and my head's a bit fuzzy, so I apologise if this is 
incorrect, but I believe this is what I was doing originally, and the 
problem was that session.models.remove([m]) dissociates all child models 
from m. So, for example, if you run this on an AtomicStructure then when 
it's added back to the new parent then the "Missing atoms" child model 
will have disappeared into the great unknown. I put together my version 
to ensure that whatever's underneath the model being moved remains 
intact.

On 2017-05-31 21:34, ChimeraX wrote:

[goddard@…]

The rename code reparents all child models.  Every model contains an id attribute which is the full chain of integer model ids of its ancestors e.g. (1,2,3) for #1.2.3, so if any parent is moved then all children must get new ids as well - they must all be reparented (even if they keep the same parent) so their id is updated and the Models class keeps a dictionary mapping id to model.  The rename code appears to do this.  If it does not work correctly please file a bug report, preferably giving an example using the rename command.

[Greg Couch]

There was a bug in the code that faked the missing entity_poly_seq table. If there were multiple chains of the same entity, duplicates were created which confused things.