search

choderalab / perses

Experiments with expanded ensembles to explore chemical space
http://perses.readthedocs.io
MIT License
181 stars 51 forks source link

Rework logic for adding ring-closure torsion biases for fused/bridging ring systems #818

Open schmolly opened 3 years ago

schmolly commented 3 years ago

One of the recurring errors I've seen (using Perses 0.8.1, in my slightly janky Docker container)

Result: the atom mapping fails, and the edge does not run

Traceback (most recent call last):
  File "/miniconda/lib/python3.8/site-packages/perses/app/setup_relative_calculation.py", line 849, in <module>
    run()
  File "/miniconda/lib/python3.8/site-packages/perses/app/setup_relative_calculation.py", line 700, in run
    setup_dict = run_setup(setup_options)
  File "/miniconda/lib/python3.8/site-packages/perses/app/setup_relative_calculation.py", line 428, in run_setup
    fe_setup = RelativeFEPSetup(ligand_file, old_ligand_index, new_ligand_index, forcefield_files,phases=phases,
  File "/miniconda/lib/python3.8/site-packages/perses/app/relative_setup.py", line 426, in __init__
    self._ligand_positions_new_solvated, self._solvent_logp_proposal = self._geometry_engine.propose(self._solvent_topology_proposal,
  File "/miniconda/lib/python3.8/site-packages/perses/rjmc/geometry.py", line 235, in propose
    logp_proposal, new_positions, rjmc_info, atoms_with_positions_reduced_potential, final_context_reduced_potential, neglected_angle_terms, omitted_terms = self._logp_propose(top_proposal, current_positions, beta, direction='forward', validate_energy_bookkeeping = validate_energy_bookkeeping)
  File "/miniconda/lib/python3.8/site-packages/perses/rjmc/geometry.py", line 400, in _logp_propose
    growth_system_generator = GeometrySystemGenerator(top_proposal.new_system,
  File "/miniconda/lib/python3.8/site-packages/perses/rjmc/geometry.py", line 2105, in __init__
    self._determine_extra_torsions(extra_modified_torsion_force, reference_topology, growth_indices)
  File "/miniconda/lib/python3.8/site-packages/perses/rjmc/geometry.py", line 2313, in _determine_extra_torsions
    p4_target_growth_index = min(tup[1] for tup in _nbr_to_growth_index_tuple if tup[1] > p1_target_growth_index)
ValueError: min() arg is an empty sequence
jchodera commented 3 years ago

Thanks for submitting this!

This code path still exists in the latest release (0.9.0), and is used unless we specify use_given_geometries=True to RelativeFEPSetup in that release. The code that fails here is trying to identify torsions in ring systems that should be used to add biasing potentials to ensure the rings will close. My guess is this can go wrong with complex multi-ring or bridged-ring systems.

Any chance you have a non-proprietary pair of compounds that could trigger this error?

schmolly commented 3 years ago

I can confirm they have been multi-ring and/or bridged-ring systems. I don't currently have a share-able example, but I can probably generate a few if that would help.

jchodera commented 3 years ago

I can confirm they have been multi-ring and/or bridged-ring systems. I don't currently have a share-able example, but I can probably generate a few if that would help.

Yes! It would be super helpful if you can come up with anything that shows the same behavior.

schmolly commented 3 years ago

Here are a couple!

image

ClC1=CC=CC(=C1)C1=CC=CC=C1

CC1CC2CC2CC1C1=CC(Cl)=CC=C1

OC1CC2CCC1(C2)C1=CC(Cl)=CC=C1

jchodera commented 3 years ago

You rock, @schmolly! Thanks so much---this is super helpful!

jchodera commented 3 years ago

I've updated the issue with a more accurate description of what needs to be fixed.

@schmolly : Can you let us know whether this is a super high priority issue to include before we release 0.9.1, or whether we could include this fix in 0.10.0?

schmolly commented 3 years ago

It's not super high priority