Repository navigation
PathMoveChange improvements - #251
Conversation
|
Basically, I don't think there's any need for what you're calling a |
Detailed BalanceI need some more help for clarification here to make sure that we talk about the same thing. Detailed balance refers to
Using the input_ensemble and output_ensemble we could (not really easy, but possible) scan the whole mover tree and compute a relative proposal probability for all possible moves and between which ensembles they move. If that matrix is symmetrizable we know that we have a stationary distribution meaning we know what portion of the time the replica is in which ensemble. This requires some more logic in each mover. In this matrix of change probabilities of moving in or between ensembles are still lots of MoverDependent Probabilities, but these are Mover Specific and should in most cases cancel (see your example #2). So that could really work. That could be future. :) Btw. How do we implement SR when we cannot remove samples from samplesets? Do we still need the ForceEnsembleChangeMover? Or do we add the Sample(trajectory=None) as a delete Sample. There is of course the possibility to let the EnsembleHopMove to keep the replicaID then it will overwrite the current sample. But I think we need to check the current implementation for that. |
Interesting, that could work. That solves the problem of default acceptance. Still, having to add an acceptance mover only for Sequential-like movers seems unintuitive to me. In QM thinking. I want a sequence of steps to depend on measurements. Then I would think that I require the results of measurements and not the experiments and do the measurements myself. Hmm, I am still not sure which way is better. Another way would be to specify the acceptance when calling the mover: the acceptor is given to all sub-movers and used. This would also make sure that you cannot mix different acceptors (would that even make sense?). I know that mixing of different acceptance schemes can introduce a bias, there is something called "Parrondo's paradox". This way we would also not have to store the acceptor in the PathMover. Only the changes would know about the used acceptor of course. |
Overall, detailed balance refers to making sure that the state of the Markov chain satisfies microscopic reversibility. Our "state" is the set of all active samples. If a move only involves one ensemble, then we have to make sure that it works at a per-ensemble level (shooting, path reversal). If it involves more than one ensemble, then we should make sure that it works on a sampleset level. RepEx does this easily; some compound moves like Minus do it by combining asymmetric moves (hop, extend), each of which, independently, does NOT satisfy detailed balance. But combined, they do. Single replica hops should satisfy detailed balance, but they only will if there are other hops which are part of the reverse process. (Similarly, extend and truncate can make a pair of moves; each of which alone does not satisfy detailed balance, but an appropriate pair does).
Not always -- only if the user behaves well when designing the move scheme. Even if you have a forward and backward hop, they might be submoves of another move, in which case they aren't actually the "reverse" of the other move. Look at the single replica example I drew, but pretend that instead of the SHS mover in B, we just had a B->A hop. It would be impossible to make this satisfy detailed balance, because it would be impossible (or at least immeasurably unlikely) have a move starting in B create the same trial that had been made by a move starting in A. (You could only satisfy detailed balance if you accounted for the condition that the SHS rejects every move that gives a new trajectory -- in other words, if the shooting has any net effect.)
We replace in the sampleset based on replica ID, not the ensemble. Replica ID never changes in SR. Only the ensemble does. (BTW, your StateSwap is not the Single Replica State Swap, which is what we normally mean by State Swap.)
This is why I was thinking of two groups of Then there's I think it is possible to hack some math and maybe add some flags to keep this all in one class. But why? Isn't it more straightforward to distinguish between bundled moves and multistep trials? Doesn't making that distinction explicitly save our users from exactly the kind of confusion we had? In the QM analogy, I think this makes complete sense. Prepare your system in some superposition state (start your move). Let it evolve for some time (generate part 1 of the trial). Let it evolve for longer time (generate part 2 of the trial). Measure now. Compare that to what would happen if you measured twice in that period. Different results -- like I said, it's quantum Zeno.
I don't think there's a problem with allowing different acceptors -- the user has to really want to do that in order to make it happen, so we shouldn't be getting in their way if they think they've got a clever idea. The default is just Metropolis for everything. I think Acceptor as a wrapper is the best idea so far. |
That was what I wanted to know :)
That was not my intent. At least, the goal was not to fix a potential non-balanced simulation, but to test whether it obeys detailed balance. That would be very nice. But I agree that this involves some more logic and coding in PathMovers which might be too cumbersome at the time.
Well, not really. It is just another way of placing the AcceptanceMovers somewhere in the mover tree. You want to put them with different kinds of SequentielMovers using an additional parameter, like while I would prefer to put the acceptor with the mover that it accepts You differentiate between bundled and multi-trial moves, while I differentiate between Movers that have an accept at the end (called real or just movers) and ones that don't (called trials). Both ways are more or less equivalent in the tree structure of movers they generate. The Acceptance will be at the same places. So I would say, that we agree on that part. It seems to have reached the philosophical state, which is a good sign :) Also, the To be honest this is not totally true, the SequentialMover (in my terms) is not a Mover but should have been called SequentialTrial, since it will not accept if you put in trials. If, however, you only put in "movers" then the SequentialTrial is also a SequentialMover. And so the only mover approach is selfconsistent.
A few questions that I think we need to sort out:
Anyway, I need to work this out a little more and see little code examples to make a decision what is clearer. The wrapper is a good idea. I like writing |
What I'm saying, though, is that I don't think it is even possible to determine that in all cases, unless you either require that all moves be made in pairs to find the reverse version (kind of like you do with snapshots, although some movers are their own reverse) or have the move scheme builder manually identify them. Note that it also is a concept that only applies at the
I think I didn't explain the distinction I'm trying to make. Let me try again. All movers are just trials. Nothing is actually accepted until the final acceptor accepts it. What I call "bundling" is not necessarily correctly called "real", because even accepted moves (during bundling) might not be "real", i.e., might not be accepted. For example, consider the following move scheme: Just because A1 was accepted part of the bundle in SeqEns doesn't mean that it will be accepted overall, because the ConditionSequential might not accept one of its later moves. Again, there are two ways to make a move that we're discussing. One is just to create a trial move, the old-fashioned way, and accept based on the appropriate probability. Those are trials, sometimes made of subtrials.
Can you explain how you plan to handle the difference in the mathematics? For a sequential trial, the correct value to return for It seems like getting this behavior out of the same class will involve adding if-statements to use completely different logic for the two cases. I would call that a sign that we really have two classes. I remark again that Given that it took us a while to make this distinction, I don't think we should assume that users will get it right away. This is why I think we should split Sequential into two classes: This also allows usage to be easier. Let's say a user wanted to make the repex-shoot-repex example I always give. If I understand correctly, in your implementation, this looks like: SequentialTrial([
MetropolisAcceptor(repex12), MetropolisAcceptor(repex23),
MetropolisAcceptor(shoot1), MetropolisAcceptor(shoot2), MetropolisAcceptor(shoot3),
MetropolisAcceptor(repex23), MetropolisAcceptor(repex12)
])In mine: BundledSequential([repex12, repex23, shoot1, shoot2, shoot3, repex12, repex23])Both should give the same behavior. Why not use the shorter syntax?
Again, in a technical sense, I might agree, but I'm pretty sure that I disgree with the general implications. There is only one place a change becomes truly "decided" or "real". Until then, it just has an acceptance probability. That probability may appear, at some point in the process, to be 100% (based on the information so far). However, it might still get rejected later, because 100% times 0% gives us 0%.
No. There is no need for this. Looking at the various diagrams I've drawn in this thread, consider where we need an "acceptor" (an oval around the move): either at the very top of the tree, or around movers in a bundling-type sequential move. If the user is using Metropolis, the user should never need to explicitly make an acceptor.
I don't understand the question. The only place acceptance actual happens is at the last step of the move. Everything else calculates a probability of acceptance.
I'm completely in support of renaming nearly everything to "Trial" ... this also gets rid of the "Generating" making some of this longer. |
|
I wrote up some stuff that includes more detailed math around the Monte Carlo discussion we’ve been having here (including the mathematical derivation of how the "bundling" moves work). It’s all in a gist. Direct link to the PDF. In order to be complete, it rehashes some pretty basic stuff. On the other hand, some of those basics are all you really need for the bundled move derivation! |
|
Quick status on this. Here are the issues I think are still unresolved:
I believe that we agreed on the first two points, but the changes aren't included in the PR yet. We're still working out details for a consensus approach to the third point. Do we want to split these off as new issues so the rest of this can get merged? I currently have 5 PRs waiting on aspects of this one, and I'm running out of code to write that doesn't require this PR to be merged. |
|
Alright. I spend a lot of time thinking about the best way implementing the possibility to seprarate acceptance steps and I think I have found a way to incorporate both approaches, but I will do that in the next PR. I agree with David in the points above and will fix this quickly so that David can continue. The renaming, I think, will mostly involve turning all movers into trials and add a few things to add acceptors, mostly a BundledSequentiel, etc. as a shortcut for almost all sequential cases and some more little building block I think are useful. With these smaller "Acceptors" we can write the BundledSequential, etc in 2 lines of code and thus keep everyhing easy to use and maintable at the same time. More infos in the PR later today...
|
|
Quick request for clarification: the update of the MC system is definitely for the next PR, but on the other points, should I merge as-is or should I wait for new commits? |
|
Let me change the in and out. The refactoring of shooting and extension is independent of the rest, so you can merge after that. |
|
nosetests passed |
|
ipython nb passed. So I think this is ready and does not include the shooting/extention refactoring! |
|
passed Travis tests -- merging! |
PathMoveChange improvements
|
Will continue the discussion about the final MoveScheme layout in a new issue. |

See #246 for detailed discussion and ideas.
.acceptedfrom samples.validto propertyMovePathChanges? #237.trials,.resultsto PMC.in_ensemblesandout_ensemblescomputation__contains__for trees.canonicalproperty : Closes Addcanonical_movefunction to PathMoveChange #243MCStep: Closes NewMCStepobject #244EnsembleHop: Closes Add support for bias in EnsembleHopMover #142PathMover.enginetoShootingMover.engine: Closes Remove global PathMover.engine #150canonicala propertycache_allfor orderparametersStateSwapMoverI kept the old names
.trialsfor all changes and.resultsfor the actual final changes.Will move some minor cleanup to a separate PR:
ensembles=[...]parameter for PathMovers : Means to use a more unified way to express the ensembles attribute for PathMovers.