Skip to content

PEP8 cleanup after #788 #790

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dwhswenson merged 4 commits into from
Oct 6, 2018
Merged

PEP8 cleanup after #788 #790

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b11f64d
PEP8 cleanup after #788
dwhswenson Jul 19, 2018
c7e3ed6
fix test errors
dwhswenson Jul 19, 2018
fbff620
Merge remote-tracking branch 'upstream/master' into interface_constra…
dwhswenson Sep 17, 2018
1804420
Merge remote-tracking branch 'upstream/master' into interface_constra…
dwhswenson Sep 30, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 26 additions & 18 deletions openpathsampling/high_level/move_strategy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@
from functools import reduce # not built in for py3

LevelLabels = collections.namedtuple(
"LevelLabels",
"LevelLabels",
["SIGNATURE", "MOVER", "GROUP", "SUPERGROUP", "GLOBAL"]
)


def most_common_value(ll):
"""
Calculates the most common value and its count. Should probably be
Expand All @@ -31,6 +32,7 @@ def most_common_value(ll):
most_common_count = counts[v]
return (most_common, most_common_count)


class StrategyLevels(LevelLabels):
"""
Custom version of a namedtuple to handle aspects of the `level`
Expand All @@ -43,16 +45,18 @@ def level_type(self, lev):
"""
if lev < 0 or lev > 100:
return None
levels = [self.SIGNATURE, self.MOVER, self.GROUP, self.SUPERGROUP,
levels = [self.SIGNATURE, self.MOVER, self.GROUP, self.SUPERGROUP,
self.GLOBAL]
distances = [abs(lev - v) for v in levels]
mindist = min(distances)
indices = [i for i in range(len(distances)) if distances[i]==mindist]
indices = [i for i in range(len(distances))
if distances[i] == mindist]
if len(indices) > 1:
return None
else:
return levels[indices[0]]


# possible rename to make available as paths.strategy_levels?
levels = StrategyLevels(
SIGNATURE=10,
Expand All @@ -62,6 +66,7 @@ def level_type(self, lev):
GLOBAL=90
)


class MoveStrategy(StorableNamedObject):
"""
Each MoveStrategy describes one aspect of the approach to the overall
Expand Down Expand Up @@ -114,7 +119,7 @@ def level(self):
@level.setter
def level(self, value):
self._level = value
self.set_replace(self.replace) # behavior of replace depends on level
self.set_replace(self.replace) # behavior depends on level

def set_replace(self, replace):
"""Sets values for replace_signatures and replace_movers."""
Expand All @@ -134,7 +139,7 @@ def get_ensembles(self, scheme, ensembles):
Regularizes ensemble input to list of list.

Input None returns a list of lists for ensembles in each sampling
transition in the network. A list of ensembles is wrapped in a list.
transition in the network. A list of ensembles is wrapped in a list.

Parameters
----------
Expand Down Expand Up @@ -201,7 +206,7 @@ def make_movers(self, scheme):
paths.PathMover or list of paths.PathMover
the movers created by this part of the strategy
"""
raise NotImplementedError #TODO: use JHP's ABCError when 302 is merged
raise NotImplementedError # TODO: use ABCError when 302 is merged


class SingleEnsembleMoveStrategy(MoveStrategy):
Expand Down Expand Up @@ -264,7 +269,7 @@ def get_init_ensembles(self, scheme):
except KeyError:
movers = []

# these are the conditions
# these are the conditions
conditions = (self.ensembles is not None
or len(movers) == 0
or self.replace_signatures or self.replace_movers)
Expand All @@ -283,7 +288,10 @@ def get_init_ensembles(self, scheme):

class ForwardShootingStrategy(SingleEnsembleMoveStrategy):
"""
Strategy for Forwardhooting only. Allows choice of shooting point selector. Useful for e.g. Constrained Interface shooting.
Strategy for Forwardhooting only.

Allows choice of shooting point selector. Useful for e.g. Constrained
Interface shooting.

Parameters
----------
Expand All @@ -299,7 +307,9 @@ class ForwardShootingStrategy(SingleEnsembleMoveStrategy):
replace : bool
whether to replace existing movers in the group; default True
"""

_level = levels.MOVER

def __init__(self, selector=None, ensembles=None, engine=None,
group="shooting", replace=True):
super(ForwardShootingStrategy, self).__init__(
Expand All @@ -311,14 +321,14 @@ def __init__(self, selector=None, ensembles=None, engine=None,
self.engine = engine

def make_movers(self, scheme):
#ensemble_list = self.get_ensembles(scheme, self.ensembles)
ensemble_list = self.get_init_ensembles(scheme)
ensembles = reduce(list.__add__, map(lambda x: list(x), ensemble_list))
ensembles = reduce(list.__add__,
map(lambda x: list(x), ensemble_list))

selector=self.selector
selector = self.selector
if type(selector) is not list:
selector = [selector] * len(ensembles)

shooters = []
for (sel, ens) in zip(selector, ensembles):
mover = paths.ForwardShootMover(
Expand All @@ -332,7 +342,6 @@ def make_movers(self, scheme):
return shooters



class OneWayShootingStrategy(SingleEnsembleMoveStrategy):
"""
Strategy for OneWayShooting. Allows choice of shooting point selector.
Expand Down Expand Up @@ -371,7 +380,6 @@ def make_movers(self, scheme):
self.engine)
return shooters



class TwoWayShootingStrategy(SingleEnsembleMoveStrategy):
"""Strategy to make a group of 2-way shooting movers.
Expand Down Expand Up @@ -543,11 +551,11 @@ def sig_error(sig, errstr=""):
# the exception if the test fails, and the assertion acts as a
# backup. This is faster than a try: except: version. see
# http://stackoverflow.com/questions/1569049/#1569618
assert(len(sig[0])==len(sig[1]) or sig_error(sig))
assert(len(sig[0]) == len(sig[1]) or sig_error(sig))
n_ens = len(sig[0])
if n_ens == 2: # replica exchange
if n_ens == 2: # replica exchange
assert(
set(sig[0])==set(sig[1]) or
set(sig[0]) == set(sig[1]) or
sig_error(sig, errstr="Not replica exchange signature. ")
)
elif n_ens == 1: # already ensemble hop (ish)
Expand All @@ -561,7 +569,7 @@ def sig_error(sig, errstr=""):
" ensembles.")

def make_movers(self, scheme):
signatures = [m.ensemble_signature
signatures = [m.ensemble_signature
for m in scheme.movers[self.from_group]]
# a KeyError here indicates that there is no existing group of that
# name: build scheme.movers[self.from_group] before trying to use it!
Expand Down
23 changes: 12 additions & 11 deletions openpathsampling/shooting.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,15 +137,15 @@ def pick(self, trajectory):
return idx



class InterfaceConstrainedSelector(ShootingPointSelector):
"""
Selects first frame outside of volume.

Parameters
----------
volume : :class:`.Volume`
defines Volume for which the first frame outside of this interface volume is found
volume : :class:`.Volume`
defines Volume for which the first frame outside of this interface
volume is found
"""

def __init__(self, volume):
Expand All @@ -154,7 +154,7 @@ def __init__(self, volume):

def f(self, frame, trajectory=None):
idx = trajectory.index(frame)
if idx==self.pick(trajectory):
if idx == self.pick(trajectory):
return 1.0
else:
return 0.0
Expand All @@ -163,15 +163,16 @@ def sum_bias(self, trajectory):
return 1.0

def pick(self, trajectory):
for idx,frame in enumerate(trajectory):
if not self.volume(frame):
break;
for idx, frame in enumerate(trajectory):
if not self.volume(frame):
break
if idx == len(trajectory)-1 and self.volume(frame):
raise RuntimeError("Interface constrained shooting move did not find valid crossing point" )

raise RuntimeError("Interface constrained shooting move did "
" not find valid crossing point")

return idx


class FinalFrameSelector(ShootingPointSelector):
'''
Pick final trajectory frame as shooting point.
Expand All @@ -188,7 +189,7 @@ def pick(self, trajectory):
return len(trajectory)-1

def probability(self, snapshot, trajectory):
return 1.0 # there's only one choice
return 1.0 # there's only one choice

def probability_ratio(self, snapshot, old_trajectory, new_trajectory):
# must be matched by a final-frame selector somewhere
Expand Down
43 changes: 25 additions & 18 deletions openpathsampling/tests/test_movestrategy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,12 @@
logging.getLogger('openpathsampling.storage').setLevel(logging.CRITICAL)
logging.getLogger('openpathsampling.netcdfplus').setLevel(logging.CRITICAL)


class MockMoveStrategy(MoveStrategy):
def make_movers(self, scheme):
return None


def find_mover(scheme, group, sig):
mover = None
for m in scheme.movers[group]:
Expand All @@ -52,13 +54,13 @@ def test_level_type(self):
class MoveStrategyTestSetup(object):
def setup(self):
paths.InterfaceSet._reset()
cvA = paths.FunctionCV(name="xA", f=lambda s : s.xyz[0][0])
cvB = paths.FunctionCV(name="xB", f=lambda s : -s.xyz[0][0])
cvA = paths.FunctionCV(name="xA", f=lambda s: s.xyz[0][0])
cvB = paths.FunctionCV(name="xB", f=lambda s: -s.xyz[0][0])
self.stateA = paths.CVDefinedVolume(cvA, float("-inf"), -0.5)
self.stateB = paths.CVDefinedVolume(cvB, float("-inf"), -0.5)
interfacesA = paths.VolumeInterfaceSet(cvA, float("-inf"),
interfacesA = paths.VolumeInterfaceSet(cvA, float("-inf"),
[-0.5, -0.3, -0.1])
interfacesB = paths.VolumeInterfaceSet(cvB, float("-inf"),
interfacesB = paths.VolumeInterfaceSet(cvB, float("-inf"),
[-0.5, -0.3, -0.1])
self.network = paths.MSTISNetwork(
[(self.stateA, interfacesA),
Expand Down Expand Up @@ -89,7 +91,7 @@ def test_levels(self):
assert_equal(strategy.replace_movers, False)

def test_get_ensembles(self):
self.strategy = MockMoveStrategy(ensembles=None, group="test",
self.strategy = MockMoveStrategy(ensembles=None, group="test",
replace=True)
scheme = MoveScheme(self.network)
# load up the relevant ensembles to test against
Expand Down Expand Up @@ -119,8 +121,6 @@ def test_get_ensembles(self):
assert_equal(ensembles[0][0], extra_ens)




class TestForwardShootingStrategy(MoveStrategyTestSetup):
def test_make_movers(self):
strategy = ForwardShootingStrategy()
Expand All @@ -132,18 +132,25 @@ def test_make_movers(self):
assert_equal(type(mover.selector), paths.UniformSelector)

def test_make_movers_with_list(self):
listofselectors = [paths.shooting.InterfaceConstrainedSelector(ens.interface) for ens in self.network.sampling_ensembles]
strategy = ForwardShootingStrategy(selector=listofselectors,
ensembles=self.network.sampling_ensembles)
list_of_selectors = [
paths.shooting.InterfaceConstrainedSelector(ens.interface)
for ens in self.network.sampling_ensembles
]
strategy = ForwardShootingStrategy(
selector=list_of_selectors,
ensembles=self.network.sampling_ensembles
)
scheme = MoveScheme(self.network)
movers = strategy.make_movers(scheme)
assert_equal(len(movers), 6)
assert_equal(len(listofselectors), 6)
for mover,sel in zip(movers,listofselectors):
assert_equal(len(list_of_selectors), 6)
for mover, sel in zip(movers, list_of_selectors):
assert_equal(type(mover), paths.ForwardShootMover)
assert_equal(type(mover.selector),paths.shooting.InterfaceConstrainedSelector)
assert_equal(mover.selector, sel)

assert_equal(type(mover.selector),
paths.shooting.InterfaceConstrainedSelector)
assert_equal(mover.selector, sel)


class TestOneWayShootingStrategy(MoveStrategyTestSetup):
def test_make_movers(self):
strategy = OneWayShootingStrategy()
Expand Down Expand Up @@ -225,7 +232,7 @@ def test_make_movers(self):
scheme = MoveScheme(self.network)
movers = strategy.make_movers(scheme)
assert_equal(len(movers), 1)
assert_equal(movers[0].ensemble_signature_set,
assert_equal(movers[0].ensemble_signature_set,
({ ens00, ens02 }, ({ ens00, ens02 })))

@raises(RuntimeError)
Expand Down Expand Up @@ -331,7 +338,7 @@ def test_different_number_input_output_ensembles(self):
input_ensembles=[ens0, ens1, ens2],
output_ensembles=[ens0, ens1]
)
assert_equal(weird_mover.ensemble_signature,
assert_equal(weird_mover.ensemble_signature,
((ens0,ens1,ens2),(ens0,ens1)))
scheme = MoveScheme(self.network)
scheme.movers['weird'] = [weird_mover]
Expand All @@ -347,7 +354,7 @@ def test_wrong_number_ensembles_in_signature(self):
input_ensembles=[ens0, ens1, ens2],
output_ensembles=[ens0, ens1, ens2]
)
assert_equal(weird_mover.ensemble_signature,
assert_equal(weird_mover.ensemble_signature,
((ens0,ens1,ens2),(ens0,ens1,ens2)))
scheme = MoveScheme(self.network)
scheme.movers['weird'] = [weird_mover]
Expand Down
Loading