BioSimSpace
BioSimSpace copied to clipboard
Add the ABFE functionality
This PR syncs the EXS/dev to this dev and the major update is the ABFE functionality.
Type of change Bug fix (non-breaking change which fixes an issue)
- Fix the unit test to have them running fine on GMX 2022.1
- Change the maxwarn to use the Gromacs unlimited option
- Correct some formatting issue with the checkpoint_file in the doc string
- Allow the BSS.FreeEnergy.Relative to recognise the conda-forge gromacs
- Allow BSS.FreeEnergy.Relative to load the temperature as float
- The velocity is generated automatically for BSS.Protocol.Equilibration
New feature (non-breaking change which adds functionality)
- Add the Github actions such that the CI will run all the unit tests
- Use pd.DataFrame and pd.Series internally in the BSS.Protocol._FreeEnergyMixin such that the more than one lambda state is supported.
- Add setLambda method to BSS.protocol.free_energy_mixin
- When supplying a Protocol with multiple lambda state, the BSS.Protocol._config could write multiple stage lambda into Gromacs mdp files
- Change the default mdp options to better suit the gmx-2022
- Allow BSS.FreeEnergy.relative to use the new lambda config
- Write test to ensure that the mdp file has been modified
- Allow the BSS.FreeEnergy.Relative to handle decoupled molecule
- Allow the BSS.Protocol.Config to edit the mdp file such that decoupled molecule could be written
- Add new method BSS.Align.Decouple to make molecule as being decoupled
- Allow BSS.mol and BSS.system to handle decoupled state properly
- Add the new BSS.FreeEnergy.RestraintSearch class to deal with ABFE restraint search.
- Allow the BSS.FreeEnergy.Relative to handle restraint
- Allow the BSS.FreeEnergy.Relative to use pd.DataFrame internally to handle the lambda values.
- Remove the duplication of the config related lines in BSS.Process.Gromacs
- Change the default
refcoord-scaling
tocom
- Allow BSS.Process.Gromacs to handle restraint
- Add new tests for the ligand-protein system
Changes (breaking change which adds functionality)
- Change the folder being created to use the index of the lambda (e.g. lambda_0, lambda_1 ... lambda_32)
- Change the default trajectory saving format to xtc from trr.
- Change the mdp options such that they are GPU compatible
Thanks for this, I should be able to merge in within the next few days.
@lohedges Thanks. Do you need to review it? In the last time when I try to merge some functionality, you have given some valuable advices? https://github.com/michellab/BioSimSpace/pull/328
Hi there, yes, I meant review as well as merge. Since it's quite a large PR it might take me a while to go through. I'll let you know if I have any comments/questions.
Cheers.
I've now done the work required to update your PR so that it is compliant with the new Sire 2023 API. I've checked that all tests pass locally and have pushed my edits to the merge-exs-devel branch.
Do you want to fetch this locally and check that everything works as expected?
With regards to this PR, I'm not sure if it's best that I submit a PR of my branch against your devel, then you re-open this PR with the updated code? Alternatively, I could still go through the code here and submit a review. Any edits/changes could then be incorporated in the the updated branch.
Cheers.
Oh, and just to note that Sire 2023 has broken SOMD. I have a fix and this will be merged in as soon as it has been tested.
@lohedges Thanks for the help. Is solving the conflict very difficult? I think the best route will be create a branch from Exscientia:devel to be Exscientia:michellab, then merge michellab:devel to Exscientia:michellab, then create a PR to merge Exscientia:michellab to Exscientia:devel. I will have a test of merge-exs-devel now, is this branch using Sire 2023?
No the conflict resolution wasn't too bad. It was just accepting a few updates your branch over devel
, renaming all Sire imports
to from sire.legacy
instead, and deleting the, now redundant, fast_system.py
file.
Yes, the branch I have made requires Sire 2023 to function. I think I forgot to update this in your CI/build scripts, so those will likely fail. (I just ran the tests locally.)
Assuming that you are okay with the branch, we could always keep this PR, then I just re-play my edits on top following the merge. I have a few review comments that I need to add first, but these mostly concern formatting, rather than the actual ABFE functionality.
Waiting for https://github.com/michellab/Sire/issues/407 to be fixed.
I have merged the dev to our dev. I wonder if you mind review this PR when you are free, please? Thank you.
@lohedges Without looking through the comments.
So I have not included anything from @annamherz's Frankenstein feature-analysis branch. I have made my own attempt at changing the analysis function as I'm more familiar with alchemlyb.
There is some WIP from @fjclark I could remove them if it makes your review easier. It is just that I fear that it will break https://github.com/Exscientia/BioSimSpace/pull/19
Ah, apologies, I was under the impression that you had merged across some of Anna's alchemlyb edits early on, I hadn't realised that you'd done your own edits. This might make things tricky when Anna's work is merged in, but we can deal with that then.
I'd leave Finlay's bit if it's working in this version. You can ignore comments on this and I'll try to copy them over to the other PR. It might be confusing as there will no longer be a diff to refer to, though.
Cheers.
Alternatively, I could try reviewing @fjclark's PR first, so that this PR would be consistent once that's merged.
@lohedges For me, it might be better if this PR gets merged first as we are preparing another several big PR now.
No problem, whatever is preferable. Ultimately this is your sandpit, so what goes in there is up to you. Any comments / suggestions made in review here are mostly for the benefit of when it comes to porting those updates into devel
.
So I have not included anything from @annamherz's Frankenstein feature-analysis branch.
It looks like this came in during the early sandpit synchronisation, i.e. when some of the edits from the feature-amber-fep branch were merged back into yours.
No worries about this, just a note to myself (everyone) that there might be some conflicts whenever feature-analysis is merged in, since it now comes from a different base.
Sorry, I see this part has been removed. Just a general comment about variable names :-)
On Fri, Nov 11, 2022 at 10:29 AM Zhiyi Wu @.***> wrote:
@.**** commented on this pull request.
In python/BioSimSpace/Sandpit/Exscientia/FreeEnergy/_relative.py https://github.com/michellab/BioSimSpace/pull/363#discussion_r1020091110 :
l = line.strip().split(':')
l = l[1].strip()
l = float(l)
Sorry, what do you mean here?
— Reply to this email directly, view it on GitHub https://github.com/michellab/BioSimSpace/pull/363#discussion_r1020091110, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAE6K3KPUFW47J5PNNP7UVDWHYNXZANCNFSM53VZZJPA . You are receiving this because you were mentioned.Message ID: @.***>
@lohedges Many thanks for the comments, I have addressed all the comments in the latest commit.