Basic Clipper session restore

[Tristan Croll]

The following bug report has been submitted:
Platform:        Linux-3.10.0-957.12.2.el7.x86_64-x86_64-with-centos-7.6.1810-Core
ChimeraX Version: 0.91 (2019-11-13)
Description
Saving/restoring with Clipper and ISOLDE active no longer causes a traceback, and the atomic model is returned on restore (any Volume instances subordinate to the SymmetryManager are lost, and will need to be reloaded by the user). The SymmetryManager restore_snapshot() just returns a generic Model. Not sure if this is desired behaviour, but in one instance I forgot to make SymmetryManager.restore_snapshot() actually *return* the Model, but to my surprise the AtomicStructure was still returned anyway - just placed at the top of the model tree. To be safe, I'm still returning the model so the restored tree looks like:

(Placeholder) Data manager (model name)
  |
  -- atomic model

Will get dev builds out with this basic behaviour, then work on doing it right.

Log:
Startup Messages  
---  
warning | 'clip' is a prefix of an existing command 'clipper'  
  
UCSF ChimeraX version: 0.91 (2019-11-13)  
© 2016-2019 Regents of the University of California. All rights reserved.  
How to cite UCSF ChimeraX  

> open 3io0 format mmCIF fromDatabase pdb structureFactors true

Summary of feedback from opening 3io0 fetched from pdb  
---  
warning | WARNING: multiple experimental reflection datasets found:  
F_meas_au, F_meas_sigma_au,  
pdbx_F_plus, pdbx_F_plus_sigma, pdbx_F_minus, pdbx_F_minus_sigma,  
pdbx_anom_difference, pdbx_anom_difference_sigma,  
intensity_meas, intensity_sigma,  
pdbx_I_plus, pdbx_I_plus_sigma, pdbx_I_minus, pdbx_I_minus_sigma  
Automatically choosing "intensity_meas, intensity_sigma".  
notes | Resolution: 3.003  
Reflection data provided as intensities. Performing French & Wilson scaling to
convert to amplitudes...  
  
3io0 title:  
Crystal structure of EtuB from Clostridium kluyveri [more info...]  
  
Chain information for 3io0  
---  
Chain | Description  
1.3/A | EtuB protein  
  
3io0 mmCIF Assemblies  
---  
1| author_and_software_defined_assembly  
  
  

> isolde start

> set selectionWidth 4

Done loading forcefield  

> save test.cxs

Session saving is not yet implemented for ISOLDE. Custom restraints will not
be saved.  

Session saving for ChimeraX-Clipper objects is not yet fully implemented. Your
atomic model will be saved, but the symmetry and volume management framework
will not.  

> open test.cxs

Summary of feedback from opening test.cxs  
---  
notes | Deleting (LIVE) 2mFo-DFc  
Deleting (LIVE) MDFF potential  
Deleting (LIVE) 2mFo-DFc_sharp_29  
Deleting (LIVE) mFo-DFc  
Deleting Crystallographic maps (3io0-sf.cif)  
  
opened ChimeraX session  




OpenGL version: 3.3.0 NVIDIA 418.87.01
OpenGL renderer: TITAN Xp/PCIe/SSE2
OpenGL vendor: NVIDIA Corporation

[pett]

Just FYI, SymmetryManager's take_snapshot() should, at a minimum, call Model's take_snapshot() and return that, preferably as a value in a dictionary (that can later be added to when SymmetryManager's take_snapshot() is completed). SymmetryManager's restore_snapshot() should call Model's restore_snapshot() with that data, getting a Model instance back, and return that.

Later, when you are actually restoring the SymmetryManager properly, instead of using Model.restore_snapshot() you will be using Model.set_state_from_snapshot() to initialize SymmetryManager's Model base class.

--Eric

[Tristan Croll]

Yep - my take_snapshot() and restore_snapshot() are doing that now (not 
the set_state_from_snapshot() part).

On 2019-11-20 18:42, ChimeraX wrote:

[Tristan Croll]

Progress update: Clipper's functionality now saves and restores successfully, but not all the precise details of the visualisation state. The model and any EM maps should come back the way they were saved, but for crystallographic maps it makes much more sense to save only the reflection data and then re-generate the maps from scratch on restore. Right now just the default maps are generated on restore... the ultimate aim is for each XmapSet to handle the defining save/restore data for all the maps it manages. I realise this goes against the "each Model handles its own save/restore" framework, but I don't see a straightforward way to do otherwise in this case.

[goddard@…]

I think a Model may have to restore itself for everything to work correctly.  The deal is that other tools might try to save in a session a reference to your Model (to do coloring, segmentation, or some other special thing with it).  If it is not restored through Model.restore_snapshot() because SESSION_SAVE = False for that Model, then I think session save will fail.  So you may wonder why Model.SESSION_SAVE even exists, good question.

[Tristan Croll]

Yeah - I already ran into just such an issue after posting. ISOLDE’s MDFFMgr is a Model that sits under the Volume instance it manages - so I’ll need to work out how to do it properly.

Also still coming to terms with how Collection instances manage their save/restore. Still, getting closer...
 

 

[pett]

You may have to go to a scheme where the XmapSets take_snapshot returns whatever object controls generating the map, along with identifying information for the map (e.g. ID number). XmapSets restore_snapshot would then get the controlling object passed in and could request that it generate the appropriate XmapSet and return that.

--Eric

[Tristan Croll]

Yep - that's more or less along the lines of what I was thinking. Each 
map just needs to store the arguments to the method that was used to 
generate it in the first place, then `restore_snapshot` can just re-call 
that. Should be reasonably straightforward, now I think about it.

On 2019-12-03 19:36, ChimeraX wrote:

[Tristan Croll]

Getting closer. Almost all Clipper and ISOLDE classes are restoring successfully - but my most recent roadblock is in saving a ProperDihedrals instance (a Collection subclass). Threw an error in the core State code about trying to create a 1043-dimensional Numpy array when copying the state data... would post the traceback but had to run out for parent/teacher interviews. Don’t suppose that rings any bells? I’m certainly not trying to do anything exotic... ‘take_snapshot()’ just returns a dict with two entries - a `Residues` and an array of names.
 

 

[goddard@…]

Never saw that kind of error as if a numpy array is being created with a shape argument that is accidentally the array elements instead of the shape.  Seems like something you can quickly track down with the traceback.

[Tristan Croll]

Seems the serialisation code doesn't know what to do with a Numpy array 
of strings. In the `Collection` in question:
{{{
     names = cvec_property('proper_dihedral_name', string, 
read_only=True)
     residues = cvec_property('proper_dihedral_residue', cptr, 
astype=convert.residues, read_only=True,
         doc='Returns a :class:`Residues` giving the parent residue of 
each dihedral. Read only')

     def take_snapshot(self, session, flags):
         data = {
             'residues': self.residues,
             'names':    self.names,
         }
         return data
}}}

... fails, but:

{{{
     def take_snapshot(self, session, flags):
         data = {
             'residues': self.residues,
             'names':    list(self.names),
         }
         return data
}}}

... saves and restores just fine. The actual restore method is 
slow-and-simple for now:

{{{
     @staticmethod
     def restore_snapshot(session, data):
         pdm = get_proper_dihedral_mgr(session)
         return ProperDihedrals([pdm.get_dihedral(r, name) for r, name in 
zip(
             data['residues'], data['names']
         )])
}}}


On 2019-12-05 18:17, ChimeraX wrote:

[Tristan Croll]

Raised the numpy-array-of-strings issue as #2664.

On 2019-12-06 09:21, ChimeraX wrote:

[Tristan Croll]

Current status: all Clipper functionality now survives save/restore. The only thing that doesn’t do it the proper way is the `AtomicSymmetryModel` which is still just replaced. Will try to rework things so it’s done properly, but it’s not hugely urgent.

In ISOLDE all restraints now save/restore, as do the rota/Rama annotators and the remote interface. The Tool/GUI itself doesn’t yet save/restore, but probably should because it does contain a few user-adjustable settings. I guess this means the `Isolde` class should also inherit from `State`?

One more minor hurdle to overcome: ISOLDE stores the set of residues to be ignored in simulations as a `Residues` instance added to the `AtomicStructure` as isolde_ignored_residues. I tried rearranging this to be a custom attribute so it will save, but that sets up a circular dependency. Probably best if I add an `isolde_ignore` flag as a custom attribute to `Residue` instead.

Working on all this has raised a related question: is it or will it be possible to do a “partial” save (e.g. of an `AtomicStructure` and its submodels)? That could be really useful for setting up an undo/redo tree for ISOLDE.
 

 

[pett]

ToolInstance inherits from State.

Using a custom boolean attribute in Residue does sound like the way to go.

There is no provision in the C++ layer for undo/redo support, barring completely clearing out the Structure and then using set_state_from_snapshot. That would not work well, because all Collections would suddenly be empty despite the corresponding objects not being "truly" deleted, and a million things showing up in the "changes" atomic trigger. Something smart (and difficult) would need to be implemented instead.

[Tristan Croll]

Save/restore seems to be quite stable for both Clipper and ISOLDE now, so going ahead and closing this.