Skip to content

New path tree#1048

Merged
dwhswenson merged 18 commits into
openpathsampling:masterfrom
dwhswenson:new_path_tree
Aug 31, 2024
Merged

New path tree#1048
dwhswenson merged 18 commits into
openpathsampling:masterfrom
dwhswenson:new_path_tree

Conversation

@dwhswenson

@dwhswenson dwhswenson commented Aug 12, 2021

Copy link
Copy Markdown
Member

This PR starts on a new path tree code. The main reason for this is that the old path trees do not fully work with SimStore because they rely on a bad design decision that SimStore is removing. Additionally, the old path trees don't work with two-way shooting in either SimStore or netcdfplus.

Furthermore, there is no unit testing for the old path trees, and they're written in high complexity code that would be difficult to test. There's also very little documentation to help anyone else maintain that code.

Plus, I think the new version has a much nicer API, especially for the most common use case (selecting replica 0):

tree = PathTree(steps)
tree.matplotlib()

Also, the API for added new plotting utilities is very straightforward: you create a class that implements two functions:

  • draw_trajectory(row, step, options)
  • draw_connector(x, bottom, top, step, options)

In these step is a namedtuple that carries the mover, the trajectory, the bool accepted, and segments, which provides information about which parts of the trajectory were new and which existed in the prior trajectory. options us the object which carries all the user-customizable plotting options, also contains a function that will get the left and right endpoints of the trajectory. row is the row number of the trajectory. x is the location of the shooting point, and bottom and top are row numbers to connect the trajectory to the shooting point origin. Examples of this are in the mpl.py file.

Unfortunately, there's a lot of really good functionality in the old path trees that will take time to replicate. I'm curious if anyone has ever actually used some of those (like changing the CSS to use different colors, especially colors that vary across a trajectory, representing the values of the snapshot, or adding text labels per snapshot).


2024 UPDATE: Overview of the contents here:

  • path_tree.py: Generate the objects that describe the path tree. This is content, as separated from presentation, but has conveniences for a few presentation modes.
  • options.py: Attempt to generalize presentation options in a way that might be valid across multiple presentation tools. This is the part of the API that is most likely to undergo massive changes.
  • plotter.py: ABC for presentation-specific plotting classes. This may see some future refactoring.
  • mpl.py: Tools for using matplotlib for presentation.
  • svg.py: Tools for using SVG for presentation.

The use modes that are expected to be supported are:

  1. Use conveniences in the PathTree object to plot:

    tree = PathTree(steps)
    tree.matplotlib()
    # or
    tree.svg()
  2. Explicitly do content and presentation in separate steps:

    # content for any presentation mode
    tree = PathTree(steps)
    # matplotlib presentation
    mpl_plotter = mpl.ThinLines()
    mpl_plotter.draw(tree)
    # svg presentation
    svg_plotter = SVGLines()
    svg_plotter.draw(tree)

@codecov

codecov Bot commented Aug 13, 2021

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.28%. Comparing base (2b764bb) to head (f4b714f).
Report is 19 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1048   +/-   ##
=======================================
  Coverage   81.28%   81.28%           
=======================================
  Files         142      142           
  Lines       15643    15643           
=======================================
  Hits        12716    12716           
  Misses       2927     2927           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sroet

sroet commented Aug 13, 2021

Copy link
Copy Markdown
Member

Unfortunately, there's a lot of really good functionality in the old path trees that will take time to replicate. I'm curious if anyone has ever actually used some of those (like changing the CSS to use different colors, especially colors that vary across a trajectory, representing the values of the snapshot, or adding text labels per snapshot).

I believe the most I have ever done with the tree representations was this least changed path notebook (It is old enough to have the wrong colors xD) (for context it was mainly used to check that all the right frames were selected)

@dwhswenson dwhswenson marked this pull request as ready for review August 27, 2024 08:19
@dwhswenson

dwhswenson commented Aug 27, 2024

Copy link
Copy Markdown
Member Author

I'm going to mark this as ready for review, because this needs to be in the 1.7 release (due out this weekend). The policy on "experimental" modules is that we don't require unit tests in place (even though it pains me not to have them). I've given this a decent bit of testing via a Jupyter notebook, which I tossed into a gist: https://gist.github.com/dwhswenson/4755b710941844e3aff488cbbe512359

The API is not completely solidified yet, especially around how to configure more custom coloring options. However, I'm reasonably confident in the parts of the API the separate the content of the path tree from its presentation as SVG, Matplotlib, or other front-end.

Things I might try to add before merge:

  • initial unit tests
  • modifications to give defaults that distinguish rejected paths from accepted paths

Things we'd like to add in the future:

  • side bar showing MC step number and decorrelated trajectories
  • hover info in SVG

Given the lack of tests and reasonably large code base, I'm going to leave this open for review as long as I can, while still getting 1.7 released. This is ready for review and comment. Without review, I will merge this after at least 80 hours, no sooner than Fri 30 Aug 17:00 GMT (19:00 my local).

@dwhswenson dwhswenson changed the title [WIP] New path tree New path tree Aug 27, 2024

@sroet sroet left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one spotted bug, one consideration, couple simplifications and some leftover commented out functionality

Comment thread openpathsampling/experimental/path_tree/mpl.py Outdated
Comment thread openpathsampling/experimental/path_tree/options.py
Comment thread openpathsampling/experimental/path_tree/options.py Outdated
Comment thread openpathsampling/experimental/path_tree/path_tree.py Outdated
Comment on lines +54 to +57
block.append(block, maximum - minimum)
minumum = 0
maximum = len(step.trajectory)
block = []

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

block is a list, so the last line of this block cancels the first one?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looking into this further; current code is working, but this makes me confused as to why (maybe this code isn't hit?) Is looks to me like that should be blocks.append(...), which would also make resetting the block (local variable) make sense.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cleaned it up a bit, but this code isn't used yet (which explains why it wasn't breaking anything)

Comment thread openpathsampling/experimental/path_tree/path_tree.py Outdated
Comment thread openpathsampling/experimental/path_tree/svg.py Outdated
@dwhswenson

Copy link
Copy Markdown
Member Author

@sroet : This can take another pass, if you have time. I think I addressed all the changes you requested.

Without review, I will merge this after at least 8 hours, no sooner than Sat 31 Aug 21:00 GMT (23:00 my local).

@sroet sroet left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One leftover doc that points to a function that doesn't seem to exist, LGTM otherwise!

Comment thread openpathsampling/experimental/path_tree/mpl.py Outdated

@sroet sroet left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dwhswenson dwhswenson merged commit bbfe5d7 into openpathsampling:master Aug 31, 2024
@dwhswenson dwhswenson mentioned this pull request Oct 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants