Skip to content

Conversation

@jaclark5
Copy link
Collaborator

@jaclark5 jaclark5 commented Jan 29, 2026

Close #89
Close #53

  • Make initial draft of BenchmarkSystem and derived class RBFEBenchmarkSystem
  • Make an example notebook to illustrate it's use and how to apply a custom network or partial charges that aren't currently represented in openfe_benchmarks.data
  • Resolve use of adaptive settings
  • Consider changes with SFE calculations

Please feel free to be nitpicky about naming and terminology.

@jaclark5 jaclark5 changed the base branch from main to data_factory January 29, 2026 23:38
@jaclark5
Copy link
Collaborator Author

@jthorton @hannahbaumann

  1. Could you point me to some resources on the protocol methods you usually use? Even if you like the format of my proposed class, that still needs to be fixed since as of now it's set to the RBFE defaults.

  2. Do you have suggestions on how to handle both the charged legs and vdW legs of the RBFE? That wasn't implemented in the previous version and I haven't some across an example of that.

Comment on lines +85 to +99
if leg == 'complex':
rbfe_protocol = RelativeHybridTopologyProtocol(settings=complex_rfe_settings)
else:
rbfe_protocol = RelativeHybridTopologyProtocol(settings=ligand_rfe_settings)

transformation = openfe.Transformation(
stateA=system_a,
stateB=system_b,
mapping=new_edge,
protocol=rbfe_protocol,
name=name
)
transformations.append(transformation)

return transformations
Copy link
Contributor

@jthorton jthorton Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See here for how you can use the adaptive settings class on the RBFE protocol to do this for you, I would just copy what it does extract the settings from the protocol adapt them and then make a new protocol and Transformation. You might need to use the isinstance check as well if all RBFE protocols should go through this function.

system.
name : str
The name / identifier of the benchmark system
network : LigandNetwork
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to make it clear that this is for RBFE only, as absolute calculations wont need this network file?

Comment on lines +218 to +224
settings.simulation_settings.time_per_iteration = 2.5 * unit.picosecond
settings.simulation_settings.real_time_analysis_interval = 1 * unit.nanosecond
settings.output_settings.checkpoint_interval = 1 * unit.nanosecond
settings.solvation_settings.box_shape = 'dodecahedron'
settings.forcefield_settings.nonbonded_cutoff = 0.9 * unit.nanometer
settings.solvation_settings.solvent_padding = 1.0 * unit.nanometer # for complex
#settings.solvation_settings.solvent_padding = 1.5 * unit.nanometer # for ligand
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No longer needed this will be done automatically by adaptive settings.

#settings.solvation_settings.solvent_padding = 1.5 * unit.nanometer # for ligand

# Only run one repeat per input json file
settings.protocol_repeats = 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea lets keep this!

"""

@staticmethod
def _protocol(forcefield) -> RelativeHybridTopologyProtocol:
Copy link
Contributor

@jthorton jthorton Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you imagine there being different scripts for the different protocol types, which act in a similar way for example, hybridtopology, non-equilibrium cycling/switching and SepTop or could we just expose a flag to chose one of these types of RBFE protocol and reuse a lot of the script?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I envisioned that the BenchmarkSystem class has most of the meat, but the RBFEBenchmarkSystem will be different in overwriting the _process_inputs and the proper _protocol methods. I'll have to see how the adaptive settings would interact with the _protocol method, but it's abstract in the base class so that it can be specific for the calculation type.


class BenchmarkSystem(ABC):
"""
Abstract base class defining the components of a and alchemical network.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something seems to be missing here.

self.ligand_dict = ofebu.process_sdf(ligands[self.partial_charge_scheme], return_dict=True)


def _process_protein(self, protein):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're currently working on adding membrane support to openfe protocols. That one would require loading the PDB into a ProteinMembraneComponent. Just as an FYI, we can deal with that when we get there =)


@staticmethod
def _protocol(forcefield) -> RelativeHybridTopologyProtocol:
"""Utility method for getting RFEProtocol settings for non charge changing transformations.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this returns settings, should this method be called _settings?

Copy link
Collaborator Author

@jaclark5 jaclark5 Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure! the class types are called protocols, but I see they are accessed with "settings"

# if abs(charge_difference) > 1e-3:
# # Error out - this shouldn't be happening
# raise RuntimeError("charge change found!")
complex_rfe_settings = protocol
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment above, these are settings and not a protocol, correct?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a difference? The settings appear to be contained in Protocol classes.

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.

Create initial plan_rbfe_network python script scripts: Create example scripts to use data dir and produce results inputs

4 participants