Skip to content

use uuid instead of 'is' for selecting ensembles in rate calculation - #919

Merged
dwhswenson merged 1 commit into
openpathsampling:masterfrom
sroet:use_uuid_for_analysis
Jun 30, 2020
Merged

dwhswenson merged 1 commit into
openpathsampling:masterfrom
sroet:use_uuid_for_analysis

Conversation

@sroet

@sroet sroet commented Jun 9, 2020

Copy link
Copy Markdown
Member

This came up on experimental storage, but should also be a bug on master.

If you load your network from one Storage object, but your steps from another (loading the same file), you might end up with 2 different instances of the same Ensemble.
The is evaluation is then too strict to handle this.

I opted to convert this to an uuid == uuid

I am having trouble setting up a test case for this, so if you have any pointers for that @dwhswenson, please let me know.

I would envision something like this should fail on master (but I don't have any workable .nc files at the moment, and couldn't quickly spot one in the test directory)

import openpathsampling as paths

s1 = paths.Storage("test.nc", 'r')
s2 = paths.Storage("test.nc", 'r')

network = s1.networks[0]

network.hist_args['max_lambda'] = { 'bin_width' : 0.005, 'bin_range' : (-3, 3) }
network.hist_args['pathlength'] = { 'bin_width' : 5, 'bin_range' : (0, 50000) }

assert network.rate_matrix(s1.steps, force=True) == network.rate_matrix(s2.steps, force=True)

@dwhswenson dwhswenson added the bugfix PRs fixing bugs label Jun 30, 2020

@dwhswenson dwhswenson 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.

This works, but note that this code path is likely to be removed in the relatively near future. The better way to do TIS analysis is the stuff in analysis.tis, and at some point a default version of that will replace the network.rate_matrix() (and related) code, which is untested and less customizable.

@dwhswenson
dwhswenson merged commit 12bc605 into openpathsampling:master Jun 30, 2020
@sroet
sroet deleted the use_uuid_for_analysis branch June 30, 2020 12:52
@dwhswenson dwhswenson mentioned this pull request Sep 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix PRs fixing bugs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants