Address detailed balance more carefully in chemical environment moves

[bannanc]

As @jchodera pointed out in the most recent google call we should only be creating and destroying "empty" atoms in chemical environments.

I think the easiest place to do this is inside the chemical environments. There are already checks there before removing atoms it would be easy to include this.

I wil plan to include this in my next pull request with environments, when I have the smirky sampler working. @cbayly13 let me know if you want this change sooner.

[jchodera]

What exactly is the proposed behavior to be added here?

What would you think about having a separation between the "chemical environment" object and a "chemical environment proposal engine" object that generates a new chemical environment proposal given an existing one? That way, we can try out a bunch of different proposal schemes to benchmark efficiency without changing the underlying chemical environment object?

[davidlmobley]

@jchodera - I believe that's the plan. @bannanc can fill in details, but Christopher is working on an entirely separate proposal engine that utilizes this framework. I think Caitlin's point in this issue is that the environment itself should probably check that an atom is empty rather than just allowing it to be deleted whether or not it's empty.

[jchodera]

Great! It might be good to hammer out a simple API for that so we can be sure that multiple groups can play with creating their own proposal engines, and so we can ensure that multiple proposal engines can coexist so they can be compared.

[davidlmobley]

Yes, @jchodera , we will eventually need an API for it. We're not actually near that stage yet -- at this point Chris is playing with IPython notebooks to figure out what types of manipulations he needs to do to the chemical environments in order to sample the types of SMIRKS patterns he wants to see coming out, then we'll work on an API spec.

(Partly, the specs for this are something we should be working out for you, but it's less urgent than the property calculator so I haven't brought up the issue yet since we need the property calculator first and if we work on the API for the sampler now, it will slow down implementation of the property calculator.)

[bannanc]

I think this behavior needs to be taken into account inside the chemical environment objects. At least the removeAtom function should check that the atom is empty.

Currently the default behavior for addAtom is to add an empty atom, although you can include optional arguments to create a decorated atom.

We are still working primarily in the framework of the sampler written for smarty, but using the chemical environment objects.

[bannanc]

@davidlmobley and @cbayly13
after in person discussions yesterday, my understanding is that the requirement for making changes to a fragment need to know the probability of reversing a move, so a decorated atom could be removed if we know the probability of adding a decorated atom back in (which would currently require multiple changes to a fragment).

For example
proposed move:
starting SMIRKS [#8:1](-[#6X4H2+0)-[#6:2]-[#1:3]
delete the unindexed atom for the final SMIRKS [#8:1]-[#6:2]-[#1:3]
In order to perform this move would have to know the probability of recreating the deleted atom with all decorators (which is probably not very probable).

I'm leaving this issue open because we will still have to figure out how to calculate these probabilities somewhere along the way.

[davidlmobley]

@bannanc - there's an important nuance. The relevant probabilities are move proposal probabilities - so it's not the probability of re-creating the deleted atom but the probability of proposing its re-creation. This means that a decorated atom could be removed, but only if it is something we would propose creating. i.e., we can delete (-[#6X4H2+0) if we can propose its creation (and we can evaluate the probability of proposing it).

Without wizardry that I won't go in to here, if we would propose creation only of ([#6X4]) for example, then we aren't allowed to delete (-[#6X4H2+0) since its proposal probability is zero.

[bannanc]

Aw! Thank you for that explanation, that is different from my understanding in our discussion yesterday.

This continues to seem like an issue for the sampler, since technically different samplers could have different creation criteria? For example, what if someone decided that a new atom could be proposed with up to 1 ORtype and 1 ANDtype so [#6X4;+0] could be added as a new atom. Then the current requirements to remove an atom with only 1 ORtype and 0 ANDtypes would be inconsistent with the created atoms. My point is should the removal criteria be checked in the chemical environment or in the sampler?

[davidlmobley]

@bannanc - there are some API issues we will have to deal with to solve this more generally, but this is basically going to need to be mostly taken care of by the move proposal machinery. Specifically, the move proposal machinery will need to figure out the probability of proposing a particular move and the probability of proposing the reverse move.

Some discussion of this is available here: #92 (comment) (and see also #110 ; key phrase is "Metropolis-Hastings").

For your purposes, what this means is that actually the environment mostly shouldn't be checking removal criteria I think -- all of this is going to need to be handled by the move proposal machinery. (Also, more sophisticated move proposal machinery could use some tricks to do fancier insertions/deletions, so, again, an argument for not doing that much checking within the environments.)

[bannanc]

I will read through the comments on the other issues.

There are a few "idiot-proof" criteria the environment checks for removing atoms. For example, indexed atoms can't be removed and "bridging" atoms cannot be removed for example in "[#6:1]~[#8]~[#1]" the oxygen can't be removed because it connects two other atoms and removing the "bridging atom" would cause issues with the network.

[davidlmobley]

@bannanc - yes, those are needed. But we can leave the "can we remove this without breaking detailed balance?" type concerns to the move proposal engine.