Session save error after deleting a Structure

[goddard@…]

The following bug report has been submitted:
Platform:        Darwin-19.5.0-x86_64-i386-64bit
ChimeraX Version: 1.1.dev202006100038 (2020-06-10 00:38:38 UTC)
Description
Saving session error after Structure closed by "delete #1" .

Log:
UCSF ChimeraX version: 1.1.dev202006100038 (2020-06-10)  
© 2016-2020 Regents of the University of California. All rights reserved.  
How to cite UCSF ChimeraX  

> open 1grl format mmcif fromDatabase pdb

1grl title:  
The crystal structure of the bacterial chaperonin groel At 2.8 angstroms [more
info...]  
  
Chain information for 1grl #1  
---  
Chain | Description  
A B C D E F G | groel (HSP60 class)  
  
1grl mmCIF Assemblies  
---  
1| author_and_software_defined_assembly  
2| software_defined_assembly  
  

> delete #1

> save test.cxs

Traceback (most recent call last):  
File
"/Users/goddard/ucsf/chimerax/ChimeraX.app/Contents/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/site-
packages/chimerax/core/triggerset.py", line 130, in invoke  
return self._func(self._name, data)  
File
"/Users/goddard/ucsf/chimerax/ChimeraX.app/Contents/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/site-
packages/chimerax/atomic/structure.py", line 69, in <lambda>  
lambda *args, qual=ses_func: self._ses_call(qual)))  
File
"/Users/goddard/ucsf/chimerax/ChimeraX.app/Contents/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/site-
packages/chimerax/atomic/molobject.py", line 1792, in _ses_call  
f(self._c_pointer)  
File
"/Users/goddard/ucsf/chimerax/ChimeraX.app/Contents/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/site-
packages/chimerax/atomic/attr_registration.py", line 43, in _getattr_  
return base.__getattr__(self, attr_name, look_in_class=base)  
File
"/Users/goddard/ucsf/chimerax/ChimeraX.app/Contents/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/site-
packages/chimerax/atomic/attr_registration.py", line 39, in _getattr_  
return look_in_class._attr_registration.get_attr(attr_name)  
File
"/Users/goddard/ucsf/chimerax/ChimeraX.app/Contents/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/site-
packages/chimerax/atomic/attr_registration.py", line 67, in get_attr  
raise AttributeError("'%s' object has no attribute '%s'" %
(self.class_.__name__, attr_name)) from None  
AttributeError: 'Structure' object has no attribute '_c_pointer'  
  
Error processing trigger "begin save session":  
AttributeError: 'Structure' object has no attribute '_c_pointer'  
  
File
"/Users/goddard/ucsf/chimerax/ChimeraX.app/Contents/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/site-
packages/chimerax/atomic/attr_registration.py", line 67, in get_attr  
raise AttributeError("'%s' object has no attribute '%s'" %
(self.class_.__name__, attr_name)) from None  
  
See log for complete Python traceback.  
  
Traceback (most recent call last):  
File
"/Users/goddard/ucsf/chimerax/ChimeraX.app/Contents/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/site-
packages/chimerax/core/triggerset.py", line 130, in invoke  
return self._func(self._name, data)  
File
"/Users/goddard/ucsf/chimerax/ChimeraX.app/Contents/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/site-
packages/chimerax/atomic/structure.py", line 69, in <lambda>  
lambda *args, qual=ses_func: self._ses_call(qual)))  
File
"/Users/goddard/ucsf/chimerax/ChimeraX.app/Contents/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/site-
packages/chimerax/atomic/molobject.py", line 1792, in _ses_call  
f(self._c_pointer)  
File
"/Users/goddard/ucsf/chimerax/ChimeraX.app/Contents/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/site-
packages/chimerax/atomic/attr_registration.py", line 43, in _getattr_  
return base.__getattr__(self, attr_name, look_in_class=base)  
File
"/Users/goddard/ucsf/chimerax/ChimeraX.app/Contents/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/site-
packages/chimerax/atomic/attr_registration.py", line 39, in _getattr_  
return look_in_class._attr_registration.get_attr(attr_name)  
File
"/Users/goddard/ucsf/chimerax/ChimeraX.app/Contents/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/site-
packages/chimerax/atomic/attr_registration.py", line 67, in get_attr  
raise AttributeError("'%s' object has no attribute '%s'" %
(self.class_.__name__, attr_name)) from None  
AttributeError: 'Structure' object has no attribute '_c_pointer'  
  
Error processing trigger "end save session":  
AttributeError: 'Structure' object has no attribute '_c_pointer'  
  
File
"/Users/goddard/ucsf/chimerax/ChimeraX.app/Contents/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/site-
packages/chimerax/atomic/attr_registration.py", line 67, in get_attr  
raise AttributeError("'%s' object has no attribute '%s'" %
(self.class_.__name__, attr_name)) from None  
  
See log for complete Python traceback.  
  




OpenGL version: 4.1 ATI-3.9.15
OpenGL renderer: AMD Radeon Pro Vega 20 OpenGL Engine
OpenGL vendor: ATI Technologies Inc.Hardware:

    Hardware Overview:

      Model Name: MacBook Pro
      Model Identifier: MacBookPro15,3
      Processor Name: 8-Core Intel Core i9
      Processor Speed: 2.4 GHz
      Number of Processors: 1
      Total Number of Cores: 8
      L2 Cache (per Core): 256 KB
      L3 Cache: 16 MB
      Hyper-Threading Technology: Enabled
      Memory: 32 GB
      Boot ROM Version: 1037.120.87.0.0 (iBridge: 17.16.15300.0.0,0)

Software:

    System Software Overview:

      System Version: macOS 10.15.5 (19F101)
      Kernel Version: Darwin 19.5.0
      Time since boot: 5 days 23:24

Graphics/Displays:

    Intel UHD Graphics 630:

      Chipset Model: Intel UHD Graphics 630
      Type: GPU
      Bus: Built-In
      VRAM (Dynamic, Max): 1536 MB
      Vendor: Intel
      Device ID: 0x3e9b
      Revision ID: 0x0002
      Automatic Graphics Switching: Supported
      gMux Version: 5.0.0
      Metal: Supported, feature set macOS GPUFamily2 v1

    Radeon Pro Vega 20:

      Chipset Model: Radeon Pro Vega 20
      Type: GPU
      Bus: PCIe
      PCIe Lane Width: x8
      VRAM (Total): 4 GB
      Vendor: AMD (0x1002)
      Device ID: 0x69af
      Revision ID: 0x00c0
      ROM Revision: 113-D2060I-087
      VBIOS Version: 113-D20601MA0T-016
      Option ROM Version: 113-D20601MA0T-016
      EFI Driver Version: 01.01.087
      Automatic Graphics Switching: Supported
      gMux Version: 5.0.0
      Metal: Supported, feature set macOS GPUFamily2 v1
      Displays:
        Color LCD:
          Display Type: Built-In Retina LCD
          Resolution: 2880 x 1800 Retina
          Framebuffer Depth: 24-Bit Color (ARGB8888)
          Main Display: Yes
          Mirror: Off
          Online: Yes
          Automatically Adjust Brightness: No
          Connection Type: Internal

PyQt version: 5.12.3
Compiled Qt version: 5.12.4
Runtime Qt version: 5.12.8

[Tom Goddard]

After using "delete #1" to close a Structure (not close #1) session save fails because the Structure.delete() method is never called and Structure.delete() cleans up some session save triggers. This only happens in 1.1 not 1.0 because I added some code to Models.close(models) that does not call "delete()" on models that are already deleted.

The underlying cause is that there is a StructureData.deleted property that says whether the C++ object was deleted and there is a Model.deleted property that says whether the Model.delete() method has been called. The Models.close() code is assuming the deleted property has not been overridden and is Model.deleted.

I am not sure what to do about this. We can't have two different definitions of "deleted" for a Model. I'd be inclined to rename StuctureData.deleted to something else like StructureData.cpp_deleted(). But I see that there are lots of other molobject.py classes that also define a "deleted" property (CoordSet, ChangeTracker, Sequence...) so changing the name of this for just StructureData is a little ugly. But maybe that is the best that can be done.

[Eric Pettersen]

I fixed this problem by guaranteeing that Models.close() uses the Model class 'deleted' property.

What I would like to do is change the atomic objects 'deleted' property to 'destroyed'. I always find It confusing to remember which name I used for the property anyway, so I have no vested interest in calling It 'deleted'. Unfortunately it is marked as a supported API for Atom and Residue (but not for other classes). This is another case where Tristan is probably the only outside developer that might be using this API right now. Tristan, do you use object.deleted much?

My suggestion would be to leave the 'deleted' property for awhile, but mark it as a deprecated API, and implement a 'destroyed' property as well and change all our code to use it. When Tristan signs off on not using the 'deleted' property anymore, give it the heave ho.

Thoughts?

[Tristan Croll]

I use it in a few places, yes. Easy enough to switch over when the time 
comes.

On 2020-06-16 18:15, ChimeraX wrote:

[goddard@…]

I think I would prefer that "deleted" be used for Models and Residue, Sequence, ..., it is easier to remember if there is only one such name. The StructureData / Model situations is exceptional.  I've only seen one place where StructureData.deleted is used (when Structure.delete() decides whether it should delete the C++ data structure).  That use is pretty special and I would just give it a special name since no external code is likely to use it -- it is almost a private method that just the Structure class needs to know about.  So I might call it structure_data_deleted.

[Eric Pettersen]

Okay. I changed Structure.delete() in an analogous fashion to Models.close() -- to explicitly use the StructureData.deleted property. Though that code was already working I felt it was more self-documenting this way and safeguards against any possible inheritance changes in the future.

[goddard@…]

Not sure I understand what the fix was.  Of course Models.close() and Structure.delete() can be changed to make sure they use the correct "deleted" property (from Model or StructureData).  But that fixes the immediate problem but just invites future problems if StructureData overrides the "deleted" property defined by Model.  Grant it takes a rare circumstance for this to cause a bug since usually both properties have the same value.  But it took me a very long time to debug this case and I do not think StructureData should override "deleted".  That property should never be overridden to give a different value.  So I am still in favor of renamed the StructureData.deleted property to StructureData.structure_data_deleted and changing any places in code that need to use that property.

[Eric Pettersen]

Here's how I see it. All atomic objects have a 'deleted' property that tells you if their C++ side has been destroyed. There is no conflict in StructureData, which does not inherit from Model. The conflict is in Structure, which inherits from both and has to decide what to do with not only the duplicate 'deleted' property but also the duplicate 'delete' method. The Structure class implements an explicit delete() method of its own that calls Model.delete() and, if necessary, StructureData.delete(). Until a moment ago, it defined the 'deleted' property to be the same as StructureData.deleted. I just switched it to be the same as Model.deleted instead.

If people want to know specifically if the C++ side of a Structure has been destroyed, they will just have to be aware that they need to explicitly reference the StructureData base class property, much in the same way as they would need to know that the property isn't called 'deleted' under your proposal.

[goddard@…]

Seems fine. Multiple inheritance, ick.  Single inheritance, ick.