Skip to content
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.

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

Add allow_self_transitions to TPSNetwork #481

Merged
merged 7 commits into from May 19, 2016

Conversation

dwhswenson
Copy link
Member

@dwhswenson dwhswenson commented May 17, 2016

Right now, if we create a TPSNetwork with from_states_all_to_all, we get literally all the possible transitions, including A->A. Typically, we don’t want the self-transitions to be allowed.

This PR adds an allow_self_transitions flag to the creation of TPSNetwork and FixedLengthTPSNetwork. This flag will default to False, in which case there are no A->A transitions.

This makes a rather significant change in the way MSTPS is handled internally: instead of saying that there is only one sequential ensemble to check (leave any state; go to any state), now it is the union of a sequential ensemble for every state (leave that state; go to any other state). This will have some negative effects on speed, especially when building the trajectory backwards. We'll see scaling something like $O(t * N_S)$, where $t$ is the length of the trajectory and $N_S$ is the number of states.

However, this is important in order to have multiple state TPS work correctly.

  • Add allow_self_transitions
  • Tests for allow_self_transitions=True
  • Tests for allow_self_transitions=False
  • Update docstrings

@dwhswenson dwhswenson self-assigned this May 17, 2016
@dwhswenson dwhswenson added this to the 1.0 milestone May 17, 2016
@dwhswenson dwhswenson mentioned this pull request May 18, 2016
7 tasks
@dwhswenson dwhswenson changed the title [WIP] Add allow_self_transitions to TPSNetwork Add allow_self_transitions to TPSNetwork May 18, 2016
@dwhswenson
Copy link
Member Author

Ready for review; passes tests.

Note: There are three ways to make a TPSNetwork:

  • TPSNetwork.__init__(initial_states, final_states)
  • TPSNetwork.from_states_all_to_all(states)
  • TPSNetwork.from_state_pairs([pairs_of_states])

The allow_self_transitions only applies to the first two. If you explicitly give the pair (stateA, stateA) in the list of allowed state pairs in the third version, then we trust that you actually meant to do that. The first two try to guess what you implicitly meant, and in that case, we assume that you don't actually want A->A transitions unless you request them directly with the allow_self_transitions flag.

@dwhswenson dwhswenson assigned jhprinz and unassigned dwhswenson May 18, 2016
@jhprinz
Copy link
Contributor

jhprinz commented May 19, 2016

looks good

@jhprinz jhprinz merged commit efe4149 into openpathsampling:master May 19, 2016
@dwhswenson dwhswenson deleted the tps_networks branch March 15, 2018 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants