[PAPRIKA] Host-Guest Free Energy Gradients

[jeff231li]

Description

This PR adds support for calculating the free energy gradients for host-guest systems with the paprika protocol.

Status

• Ready to go

[jeff231li]

Question: I noticed that the code in _evaluate_energies converts the potential energy to pint units but keeps it in kilojoule/mol. So the gradient I'm getting for energy parameters like LJ_epsilon becomes kilojoule/kilocalorie. What is the reason for keeping the potential energy in kilojoule/mol?

openff-evaluator/openff/evaluator/protocols/openmm.py

Lines 117 to 122 in ef37ac8

	potentials[frame_index] = potential_energy.value_in_unit(
	simtk_unit.kilojoule_per_mole
	)
	reduced_potentials[frame_index] = unreduced_potential * beta
	potentials *= unit.kilojoule / unit.mole

[codecov]

Codecov Report

Merging #342 (35828bf) into master (94c3703) will decrease coverage by 0.81%.
The diff coverage is 45.50%.

[SimonBoothroyd]

Should bound_window_index and unbound_window_index be lists, or can they just be single values? I think if they can be single values this will simplify _paprika_build_end_states_protocol by a fair bit!

[jeff231li]

I set bound_window_index and unbound_window_index as lists because template_values from ProtocolReplicator requires a list as input. Are you referring to setting template_value and template_index instead?

openff-evaluator/openff/evaluator/workflow/schemas.py

Lines 136 to 138 in e55c86b

	def apply(
	self, protocols, template_values=None, template_index=-1, template_value=None
	):

[dotsdl]

Thanks @jeff231li! I've added this to my queue to review within the next week or so. Please ping if you require it more urgently!

[dotsdl]

@SimonBoothroyd I'm reviewing this PR this week; are there any areas of concern preventing merge? I'll prioritize getting the CI to pass first.

[SimonBoothroyd]

@SimonBoothroyd I'm reviewing this PR this week; are there any areas of concern preventing merge? I'll prioritize getting the CI to pass first.

I think the implementation looks like a great first pass, but currently contains large areas where code either duplicates already present functionalities (ComputePotentialEnergyGradient) or which can be significantly reduced (the schema definitions) which will take some careful thinking through.

Unfortunately I don't have the time at the moment to look over this and suggest the changes which will be required to be merge, so it may be some time before this will be able to be merged - my sincerest apologies for this delay!