Synchronise with Exscientia remote #183

lohedges · 2023-10-12T15:14:38Z

This PR synchronises devel with the Exscientia remote.

I confirm that I have merged the latest version of devel into this branch before issuing this pull request (e.g. by running git pull origin devel): [y]
I confirm that I have permission to release this code under the GPL3 license: [y]

Suggested reviewers:

Backport fix from PR #125

Backport fixes from PR #128

Update Sandpit_exs.yml Signed-off-by: Zhiyi Wu <zwu@exscientia.ai> Update Sandpit_exs.yml Signed-off-by: Zhiyi Wu <zwu@exscientia.ai>

Sync with the BSS 2023.3

Co-authored-by: William (Zhiyi) Wu <zwu@exscientia.co.uk>

This reverts commit f328386.

Co-authored-by: Lester Hedges <lester.hedges@gmail.com>

Co-authored-by: William (Zhiyi) Wu <zwu@exscientia.co.uk>

Merge 0.9.2 to the devel

Co-authored-by: William (Zhiyi) Wu <zwu@exscientia.co.uk>

… be the root directory (#28) Co-authored-by: William (Zhiyi) Wu <zwu@exscientia.co.uk>

Co-authored-by: William (Zhiyi) Wu <zwu@exscientia.co.uk>

chryswoods · 2023-10-12T15:52:33Z

.github/workflows/Sandpit_exs.yml

@@ -35,12 +35,10 @@ jobs:

      - name: Install dependency
        run: |
-          mamba install -c conda-forge -c openbiosim/label/main biosimspace python=3.8 ambertools gromacs "sire=2023.2.2" alchemlyb pytest pyarrow "typing-extensions!=4.6.0" openff-interchange pint=0.21 "rdkit<2023" "jaxlib>0.3.7" tqdm
+          mamba install -c conda-forge -c openbiosim/label/main biosimspace python=3.10 ambertools gromacs "sire=2023.3" "alchemlyb>=2.1" pytest openff-interchange pint=0.21 rdkit "jaxlib>0.3.7" tqdm


Is this the right version for sire? Should it be 2023.3.2 if they want to pin to the last major release, or do they want 2023.4.0?

Good point. I'll check what's on their current branch.

chryswoods

Code looks good, subject to the query about the sire version.

The test failure for Linux looks weird. Why would OpenMM move the molecule only for that platform?

I'm happy to approve - good luck with the release :-)

lohedges · 2023-10-12T18:03:23Z

The CI script is exactly what is in their fork. @xiki-tempula: Just checking this is what you want. As for the OpenMM failures, I think these are the position restraint issues that I was talking about. The merge must have overwritten my edits.

lohedges · 2023-10-12T18:10:52Z

There is also a failure due to be guarding on Windows which is correct in our devel. I don't really like this way of synchronisation since I have to assume that the Exs fork is the "truth", which means that fixes that I've made are sometimes overwritten. (I can't preferentially take my edits.) The correct method is for me to PR to them, but no-one seems to want this approach.

This is not reproducibly satisfied on all platforms and Python variants.

lohedges · 2023-10-12T18:31:37Z

The OpenMM position restraint failure is intermittent, but seems to be a particular issue on Linux Python 3.10. I'm not sure why/how they chose their distance tolerances (they are different for each protocol) but increasing the one for minimisation fixes things.

chryswoods

👍

xiki-tempula · 2023-10-13T08:12:04Z

Thanks for the PR.
Yes, we are using Sire 2023.3.2 now but after the sync, we would like to go to Sire 2023.4.
@janjoswig Jan do you mind have to look to see if the position restraint is still fine after the sync?

janjoswig · 2023-10-24T13:38:17Z

Sorry for being late here. I noticed the issue as well that the position restraints apparently are not working in the same way depending on the platform and it still puzzles me. I just confirmed that the code is still working locally on MacOS and Linux and that atoms move without restraints and stay put with restraints. The reason for the different tolerances for each protocol is basically that these were the necessary thresholds to make the pipeline pass as well.

lohedges · 2023-10-24T15:52:34Z

No problem. I'm pleased it's something that you've noticed too. I haven't had time to debug it fully and it's not completely reproducible anyway (it only fails on the one platform intermittently). As such, I've just adjusted the tolerance for that one particular test.

lohedges and others added 20 commits July 3, 2023 18:25

Backport fix from PR #125. [ci skip]

605e2d1

Merge pull request #126 from OpenBioSim/backport_125

72022fe

Backport fix from PR #125

Backport fixes from PR #128. [ci skip]

cbfef2e

Merge pull request #129 from OpenBioSim/backport_128

511cb6b

Backport fixes from PR #128

Fix the extract in the gmx.process

d9c37e6

Update Sandpit_exs.yml Signed-off-by: Zhiyi Wu <zwu@exscientia.ai> Update Sandpit_exs.yml Signed-off-by: Zhiyi Wu <zwu@exscientia.ai>

Merge pull request #17 from Exscientia/feat_2023.3

0c7601f

Sync with the BSS 2023.3

Fixed squash and unsquash in the case of explicit dummies (#18)

64183f8

Make the saveMetric method more robust (#19)

1f1802a

Co-authored-by: William (Zhiyi) Wu <zwu@exscientia.co.uk>

Merge some recent bugfixes (#24)

f328386

Revert "Merge some recent bugfixes (#24)"

24bbaa0

This reverts commit f328386.

Added some fixes to the solvation (#25)

f9bed33

Co-authored-by: Lester Hedges <lester.hedges@gmail.com>

Fixed PBC fixing when unsquashing for dummy atoms (#20)

f1f5276

Have amber process write binary RST file instead of text rst7 file (#22)

55bb6fc

Co-authored-by: William (Zhiyi) Wu <zwu@exscientia.co.uk>

remove skip

591a535

fix import

e82f367

Merge pull request #26 from Exscientia/feat_merge_0.9.2

05667a2

Merge 0.9.2 to the devel

Fix test in devel (#27)

d5e28e8

Co-authored-by: William (Zhiyi) Wu <zwu@exscientia.co.uk>

Fix the tests such that it no longer require the working directory to…

20da673

… be the root directory (#28) Co-authored-by: William (Zhiyi) Wu <zwu@exscientia.co.uk>

Add a function for marking the alchemical ion (#30)

3c4a23d

Co-authored-by: William (Zhiyi) Wu <zwu@exscientia.co.uk>

Merge remote-tracking branch 'exscientia/devel' into sync_exscientia

2d4d01a

lohedges added the exscientia Related to work with Exscientia label Oct 12, 2023

lohedges added this to the 2023.4.0 milestone Oct 12, 2023

lohedges temporarily deployed to biosimspace-build October 12, 2023 15:14 — with GitHub Actions Inactive

lohedges had a problem deploying to biosimspace-build October 12, 2023 15:14 — with GitHub Actions Failure

lohedges temporarily deployed to biosimspace-build October 12, 2023 15:14 — with GitHub Actions Inactive

lohedges had a problem deploying to biosimspace-build October 12, 2023 15:14 — with GitHub Actions Failure

chryswoods reviewed Oct 12, 2023

View reviewed changes

chryswoods previously approved these changes Oct 12, 2023

View reviewed changes

lohedges added 2 commits October 12, 2023 19:28

Increase distance tolerance.

b9f1c84

This is not reproducibly satisfied on all platforms and Python variants.

Guard tests against missing packages.

4cceb23

lohedges dismissed chryswoods’s stale review via 4cceb23 October 12, 2023 18:29

lohedges temporarily deployed to biosimspace-build October 12, 2023 18:29 — with GitHub Actions Inactive

chryswoods approved these changes Oct 13, 2023

View reviewed changes

lohedges merged commit a203510 into devel Oct 13, 2023
5 checks passed

lohedges deleted the sync_exscientia branch October 13, 2023 09:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Synchronise with Exscientia remote #183

Synchronise with Exscientia remote #183

lohedges commented Oct 12, 2023

chryswoods Oct 12, 2023

lohedges Oct 12, 2023

chryswoods left a comment

lohedges commented Oct 12, 2023

lohedges commented Oct 12, 2023

lohedges commented Oct 12, 2023

chryswoods left a comment

xiki-tempula commented Oct 13, 2023

janjoswig commented Oct 24, 2023

lohedges commented Oct 24, 2023

Synchronise with Exscientia remote #183

Synchronise with Exscientia remote #183

Conversation

lohedges commented Oct 12, 2023

Suggested reviewers:

chryswoods Oct 12, 2023

Choose a reason for hiding this comment

lohedges Oct 12, 2023

Choose a reason for hiding this comment

chryswoods left a comment

Choose a reason for hiding this comment

lohedges commented Oct 12, 2023

lohedges commented Oct 12, 2023

lohedges commented Oct 12, 2023

chryswoods left a comment

Choose a reason for hiding this comment

xiki-tempula commented Oct 13, 2023

janjoswig commented Oct 24, 2023

lohedges commented Oct 24, 2023