choderalab / perses

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

Duplicate instances of `self._setup_complex_phase()` in relative_setup.py #1219

Open zhang-ivy opened 1 year ago

zhang-ivy commented 1 year ago

I noticed that self._setup_complex_phase() is called twice.

zhang-ivy commented 1 year ago

Iván notes that this is not super high priority since we are in the midst of overhauling the API anyway

ijpulidos commented 1 year ago

Yes,the comment on https://github.com/choderalab/perses/blob/ae5580173e321cc85014a2a780080b9b546f18bf/perses/app/relative_setup.py#L329 explains why we needed that at that instance but, as discussed, we can quickly investigate what happens if we remove the second call, since that seems redundant and it shouldn't be doing anything new.

As a reference, when I worked in the refactor for the NEQ cycling protocol, the RelativeFEPSetup.__init__ method of is more organized and it's only setting up the phases at the end of it, as per https://github.com/choderalab/perses/blob/faac03085376d8aace61d41e8b53d1e8e6e2f035/perses/app/relative_setup.py#L349 (just to keep in mind).