Add DSSP support for trajectories

[PardhavMaradani]

Hi Brady, this PR adds DSSP support for trajectories that you mentioned in this comment and pointed to an existing issue #872. Instead of an annotation, I think it would be better to have this as a direct import option for trajectories (both though GUI and API) - it might be helpful later too when everything moves to an MDA backend. This should fix #872

Here are some before and after examples:

Before	After

For the API, there is a new use_dssp param and for the GUI, there is a new checkbox as follows:

The DSSP example from the user guide shows how an average value can be used as well. Using this (which is the default) leads to a more smoother animation as these don't change per frame. This is a runtime option that can be changed from the API using dssp_type attribute in the trajectory or as below from the GUI (The GUI option only shows when there is valid dssp run info):

I added a couple of tests (with and without dssp) for basic validations.

One issue I ran into that you probably know more than me why it happens: From the GUI, the sec structure only show up after a frame change. It is the same from the API if the style is created without any selection (eg: t = mn.entities.Trajectory(u, use_dssp=True).add_style("cartoon") requires a frame change to see the results, but for t = mn.entities.Trajectory(u, use_dssp=True).add_style("cartoon", selection="all"), this shows up right away in the viewport.

[BradyAJohnston]

I think this mostly nails it straight up.

Only real change is I think we should allow for someone to enable or disable the SS computation after import. This will require a new property on the object, and to be able to handle doing the setup for the SS after import if not previously set up on import.

[PardhavMaradani]

Only real change is I think we should allow for someone to enable or disable the SS computation after import.

I did think about having this as a dynamic option like you suggested, but wasn't sure how intensive the analysis run could be, etc - so thought it'd be best to leave it at initial import. If we are making this true by default, one other option is to include a "None" option to the dssp_type enum (i.e., none, average, per_frame) - that way, it can be easily turned off (ie., not show up) and on. Currently, the actual analysis run is only done once and just the results get used in per-frame changes. This would keep things far simpler. What do you think about this option? Thanks

[BradyAJohnston]

Oh I didn't realise it was all being run on import. The "ideal" would be that it runs on frame change (not sure how possible this is) - so that we are only ever computing what is currently relevant (and could support streaming trajectories also).

Depends on how feasible this is and what kind of overhead is associated with calling the analysis each frame instead of once on initial import.

I think your suggestion of adding the None to the enum is probably better than us having another boolean property and the enum property.

[PardhavMaradani]

The "ideal" would be that it runs on frame change (not sure how possible this is) - so that we are only ever computing what is currently relevant (and could support streaming trajectories also)

The dssp run can be done for specific frames - i.e., on frame change for us, but then, we'll not have the concept of averaging because that requires a run across the whole trajectory. It also introduces an analysis run every frame change and if we make this the default, it could make scrubbing the timeline etc a bit laggy. I also tried out for streaming trajectories, but couldn't get this to work yet (even though there is frame slicing support) - seems like this is common for most other analysis stuff too that depend on n_frames.

adding the None to the enum is probably better than us having another boolean property and the enum property.

I wasn't thinking of removing the original use_dssp boolean - that would still determine whether the dssp analysis is run on import (and default to True as you suggested). The enum is the runtime option that has an additional None to not show the sec struct if needed. This same enum could also be brought into the n-panel as well. Is that ok?

[PardhavMaradani]

Hi Brady, the above commit is the closest we can get to "ideal" case.

I also figured out why the sec structures were not showing up until a frame change when importing a trajectory from the GUI or from the API without an explicit selection string. (the issue I described at the end of the initial comment). The reason for this is that an update_entities is needed after the node tree is setup after adding a style for the calculations to run and take effect. In the call through API with explicit selection string (t = mn.entities.Trajectory(u).add_style("cartoon", selection="all")), the selection phrase causes the addition of a new selection in the global selections list, which calls update_entities as part of the property update callback.

Here is a summary of the changes:

• The DSSP analysis object is now setup by default for trajectories. For trajectories that don't have enough info (like some examples in tests that didn't have names and resids) this fails gracefully and they continue to work as before
• There is a single enum called dssp that determines how the results show up. The values are per-frame, average and none. The default now is per-frame, which runs the analysis (the actual .run(...)) per frame
• When the user picks average, a full .run() is performed at that time
• When the results of a full run are available (above point), these essentially serve as cached data, so the per-frame setting just uses the values from these instead of running the per-frame analysis again
• Because of the issue mentioned above, I changed the default from None to "all" for style selection param. This is a slightly disruptive change. The effect of this is that a new selection is created and a corresponding named attribute node is added to the node tree. This required changes to the tests which were checking for the exact number of nodes after a style branch is created. One good thing about this is that this will make it easier to expose the selection for each of the styles (as we discussed in the style swap PR - this was one of the issue I pointed out there).
• The sec_struct calculation had to be skipped from picking because it causes a recursive loop. I didn't see any other way to address this

[PardhavMaradani]

Forgot to add a few rough performance measurements I made earlier in my setup:

• Previously, tests in test_trajectory.py took around 16 seconds
• With the current changes and the default of per-frame, the same tests take 40 seconds
• With the current changes and dssp option none, the same tests take 25s
  • The additional time can be accounted for the DSSP setup that happens during trajectory init
• With the current changes and dssp option full, the same tests take 35 seconds
  • Though this might seem counter intuitive, I think this can be attributed to several frame changes in the tests
• I am not sure if the Peptide DSSP option in the Cartoon style is accurate (it shows a very small selection), but with that turned on for the trajectory animation, in my setup, I see an fps of around 1.8, where as with the MDA one, it is the full 24 fps (for the regular PSF, DCD universe)

[PardhavMaradani]

The current DSSP Analysis Class unfortunately doesn't work with streaming trajectories. There is however a standalone assign function that can be used to get the sec structs, but the changes are a bit more involved. They seem to work fine for some simpler cases, but requires more testing, etc. I think it is best to handle those as a separate PR for the StreamingTrajectory entity later. Thanks

[BradyAJohnston]

I am not sure if the Peptide DSSP option in the Cartoon style is accurate

You are right that this has broken. I'm not sure when but it seems like it has been broken for a while & I haven't noticed it. I've got a version that is working again (in the branch you have seen) that is about 15x faster than what was previous running (and not working).

Given potential performance hits that you have detailed, it does make me think that maybe instead we should have SS computation disabled by default. Whether we still compute and store sec_struct on the first frame might still be a good idea, but then have re-computing the SS an option that a user has to enable. That way if someone isn't using it (just using sphere style) it won't affect performance for them.

I've seen you PR about the current frame iterator which should help us here. Additionally I've opened a PR that will help speed up SS computation : MDAnalysis/mdanalysis#5182

[PardhavMaradani]

Hi Brady, I changed the default to none. Thus, this will not change any existing behavior and even the DSSP class is not initialized. However, this can be changed at runtime and this will work seamlessly. Thanks

[PardhavMaradani]

Here is the regular psf/dcd viewport animation using the mda dssp computed average values:

psf-dcd-dssp-avg-animation.mp4

[BradyAJohnston]

Looks like something strange is happening with the cartoon style - likely an issue with the node and not this PR. I'll have a look into it.

[PardhavMaradani]

Looks like something strange is happening with the cartoon style - likely an issue with the node

Hi Brady, just fyi that I am using the node_data_file.blend from here. On a related note, enabling the Peptide DSSP option in the style shows nothing at all - might be because this is still work-in-progress. I don't know much about this to be of additional help. Thanks

import MDAnalysis as mda
import molecularnodes as mn
from MDAnalysis.tests.datafiles import DCD, PSF

u = mda.Universe(PSF, DCD)
canvas = mn.Canvas()

t = mn.entities.Trajectory(u).add_style("cartoon")

[BradyAJohnston]

Ah OK - looking at it just from this branch it all works fine so that's something to worry about for that branch:

[BradyAJohnston]

I think we could organise it a bit more maybe with a helper class for DSSP related stuff, and allow a user to change the window over which we average SS computed frames.

Additionally when changing from "per-frame" to "average" and the DSSP has to be run on the entire trajectory - some kind of feedback to the user that Blender might freeze for some time (especially on larger systems) would be good. An enum isn't usually something you might expect to freeze the program when you change. Not sure what the best approach might be though.

[PardhavMaradani]

and allow a user to change the window over which we average SS computed frames

Hi Brady, given the requirements are evolving, could we agree upon how this will eventually look (from both the GUI and API perspective) before I go ahead and make any further changes? Here is what I have in mind that could potentially address all the issues discussed so far:

• Have a separate DSSP panel in the n-panel
• Have an Initialize button (operator) in the panel
  • This only shows up as long as DSSP is not initialized
  • No other options are shown along with it - they show up automatically after initialization
  • This would be equivalent to disabling DSSP by default and allowing users to enable it if they need cartoon style, etc
  • Both the GUI operator and API will use a common initialize method (<trajectory>.dssp.initialize()) - using a helper class for all DSSP stuff
• Once intitialized, show a drop down with the following display options:
  • None
    • This option will not show any SS
  • Per Frame
    • This option will invoke a dssp run for the current frame unless a full run across the trajectory (see 'Trajectory Average' below) is available
    • Per frame SS are shown
  • Average of +/- N Frames
    • This option will show another integer property selection below the drop down (min 1)
    • A dssp run for the current frame +/- n frames is run for every frame change (sliding window clamped at frame beginning and end)
    • No caching of the results
    • Does not show up for streamed trajectories
  • Trajectory Average
    • This option will invoke a full dssp run and cache the results
    • The initial idea for the mean came from the user guide example, which was run on the whole trajectory
    • We could potentially add a threshold option as described here in the future
    • Does not show up for streamed trajectories
• An Apply button below the above options
  • This is disabled if none of the options change
  • An explicit click of this is required after changing the drop down or int selection values above
    • For streamed trajectories, this might seem like an unnecessary step, but this will keep it consistent
  • This invokes the operator which calls the common display(...) method used by the API as well
  • This is where the the full run or the initial average +/- n frames run etc happen - similar to the Fetch button for PDB download, this stays clicked till the blocking op is complete (this is the best we can get - adding another popup for confirmation etc, will make it unnecessarily complex)
    • We will obviously not do a full run if already available, etc
• Since the sec_struct calculation is not pickled, the same path from initialization again will work for the save and restore case

Your thoughts? Thanks

[PardhavMaradani]

Hi Brady, I went ahead and made changes as per the previous comment. A few minor changes from there - mostly for better usability, but the rest are the same. You could give this a try. Here is a video that shows the new options:

trajectory-dssp-manager-new.mp4