choderalab / perses

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

Error with spectator ligands #1232

Closed arjun-nufc closed 8 months ago

arjun-nufc commented 1 year ago

I'm getting the following error with Perses 0.10.2 running a test on a system with a spectator ligand:

AttributeError: 'RelativeFEPSetup' object has no attribute '_spectator_md_topologies'
Traceback (most recent call last):
  File "/cluster/home/narayaa2/.conda/envs/perses_0.10.2/bin/perses-cli", line 10, in <module>
    sys.exit(cli())
             ^^^^^
  File "/cluster/home/narayaa2/.conda/envs/perses_0.10.2/lib/python3.11/site-packages/click/core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/cluster/home/narayaa2/.conda/envs/perses_0.10.2/lib/python3.11/site-packages/click/core.py", line 1078, in main
    rv = self.invoke(ctx)
         ^^^^^^^^^^^^^^^^
  File "/cluster/home/narayaa2/.conda/envs/perses_0.10.2/lib/python3.11/site-packages/click/core.py", line 1434, in invoke
    return ctx.invoke(self.callback, **ctx.params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/cluster/home/narayaa2/.conda/envs/perses_0.10.2/lib/python3.11/site-packages/click/core.py", line 783, in invoke
    return __callback(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/cluster/home/narayaa2/.conda/envs/perses_0.10.2/lib/python3.11/site-packages/perses/app/cli.py", line 109, in cli
    run(yaml_filename=yaml, override_string=override)
  File "/cluster/home/narayaa2/.conda/envs/perses_0.10.2/lib/python3.11/site-packages/perses/app/setup_relative_calculation.py", line 831, in run
    setup_dict = run_setup(setup_options)
                 ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/cluster/home/narayaa2/.conda/envs/perses_0.10.2/lib/python3.11/site-packages/perses/app/setup_relative_calculation.py", line 516, in run_setup
    fe_setup = RelativeFEPSetup(ligand_file, old_ligand_index, new_ligand_index, forcefield_files, phases=phases,
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/cluster/home/narayaa2/.conda/envs/perses_0.10.2/lib/python3.11/site-packages/perses/app/relative_setup.py", line 333, in __init__
    self._setup_complex_phase()
  File "/cluster/home/narayaa2/.conda/envs/perses_0.10.2/lib/python3.11/site-packages/perses/app/relative_setup.py", line 616, in _setup_complex_phase
    for i, spectator_topology in enumerate(self._spectator_md_topologies,1):
                                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'RelativeFEPSetup' object has no attribute '_spectator_md_topologies'
arjun-nufc commented 1 year ago

I should have left more context above. This is a system i had successfully run with 0.10.1, and i used the same yaml file that worked with that version of Perses, from the same directory with the same input files and such.

arjun-nufc commented 1 year ago

it looks like self._spectator_md_topologies is first being defined on line 360 of perses/app/relative_setup.py, but is being used in self._setup_complex_phase which is called on line 333.

arjun-nufc commented 1 year ago

In contrast, in 0.10.1, self._spectator_md_topologies is being defined on line 330 and self._setup_complex_phase is called on line 379. So I think the spectator handling has to be moved before _setup_complex_phase is called ?

mikemhenry commented 1 year ago

Thank you for this bug report, we will get it fixed before we ship 0.10.3 which will be our last 0.10.x release (unless we find some nasty bugs) before we switch up our API to something that is easier to maintain.

arjun-nufc commented 1 year ago

thanks @mikemhenry !

ijpulidos commented 1 year ago

Thanks for this report. This was introduced by e2ecec6cda982234393dbdca96d38b42f2cc7b7f and it seems this is something that we are not catching in out tests. @arjun-nufc would you be able to provide an example script that is failing so we can have a starting point to make it a test? (It doesn't have to be complete or minimal, just an example we can reproduce so we can make it into a test).

The fix could be just to call the ._setup_complex_phase method after handling the spectator molecules parameters.

mikemhenry commented 1 year ago

With our 0.10.3 release this issue should be fixed, @arjun-nufc Can you confirm things are working with the latest version? Thanks!

arjun-nufc commented 8 months ago

This looks fixed @mikemhenry and @ijpulidos , thank you !