-
Notifications
You must be signed in to change notification settings - Fork 51
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 contribution where we perturb from unmodified System objects to alchemical versions #32
Comments
It looks like this will require a change in the interface. We currently have # Initialization
alchemical_engine = alchemical_engine_library.AlchemicalEliminationEngine(alchemical_metadata)
ncmc_engine = ncmc_switching.NCMCEngine(temperature=temperature, timestep=switching_timestep, nsteps=switching_nsteps, functions=switching_functions)
...
# annihilation of old atoms
old_alchemical_system = alchemical_engine.make_alchemical_system(system, top_proposal, direction='delete')
[ncmc_old_positions, ncmc_elimination_logp] = ncmc_engine.integrate(old_alchemical_system, positions, direction='delete')
...
# creation of new atoms
new_alchemical_system = alchemical_engine.make_alchemical_system(top_proposal.new_system, top_proposal, direction='create')
[ncmc_new_positions, ncmc_introduction_logp] = ncmc_engine.integrate(new_alchemical_system, geometry_proposal.new_positions, direction='create') Here are some possibilities:
# Initialization
ncmc_engine = AlchemicalNCMCEngine(temperature=temperature, timestep=switching_timestep, nsteps=switching_nsteps, functions=switching_functions)
...
# annihilation of old atoms
[ncmc_old_positions, ncmc_elimination_logp] = ncmc_engine.integrate(top_proposal, positions, direction='delete')
...
# creation of new atoms
[ncmc_new_positions, ncmc_introduction_logp] = ncmc_engine.integrate(top_proposal, geometry_proposal.new_positions, direction='create') I actually like Option 3 even if it means a bit more refactoring, since it reduces the number of arguments that must be specified multiple times and eliminates some intermediate variables ( |
Options 2 and 3 are both fine, IMO. Re: option 2, I think that now that we have a convenience class for topology proposals that include both new and old systems, requiring Re: option 3, I think this is probably the best option. Modular code is great, though as you've noted before, the I'm +1 on option 3. |
Great. I'll try implementing option 3 and see if I run into any showstoppers. |
Looks like this is resolved. |
For large systems, it will become critical to include the contribution where we perturb between the physical
System
objects and alchemically-modified versions of theSystem
object.This is just a reminder to myself to include this into the NCMC scheme.
The text was updated successfully, but these errors were encountered: