-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
alchemical_introduction tutorial is broken on try.openbiosim.org #5
Comments
Yes, the |
is then best option for the time being to update the tutorials installation instructions and rebuild the notebook container to pin pymbar<4 so we have a working BSS.FreeEnergy.Relative.Analysis over the Summer ? |
Well, the installation instructions currently specifies If I do:
I get:
This is resolvable if I add |
I think that should work? Alchemlyb was mainly added for the AMBER analysis, but as there is currently no native/alchemlyb analysis in BSS used for the tutorials downgrading alchemlyb shouldn't cause any issues I think. |
Yes - you lost the It would be easy to switch back to the old notebook image that didn't have The sustainable solution is to do the porting work in |
@chryswoods - we plan to deprecate use of analyse_freenrg in BioSimSpace and switch to alchemlyb >2 so porting analyse_freenrg would only be a temporary fix. @anna - the intro tutorial uses BSS.FreeEnergy.Relative.analyse in section 5.1 then again in section 1 of the ABFE analysis tutorial. |
Ok - does this mean removing the mbar parts of analyse_freenrg? We still need to keep the original non-mbar code because it is used to calculate free energies for waterswap simulations. |
Since it seems to have been a quick fix, I've updated |
Great, glad it was easy. Are there any tests for the waterswap analysis anywhere? I'm just wondering if there are parts other parts of the code that aren't covered by the tests above that would need updating to the new API too. |
We wouldn't touch the analyse_freenrg code in sire, we would just stop using it when calling BSS.FreeEnergy.Analyse. It will still be possible to use analyse_freenrg from the command line. |
As far as I could tell from searching "pymbar", |
yes - this uses the native analysis only, so is currently dependant on the analyse_freenrg from sire |
So changing the environment build instructions to
allows me to call without errors BSS.FreeEnergy.Relative.analyse in section 5.1 and to call BSS.FreeEnergy.AlchemicalFreeEnergy.analyse in 02_analysis_abfe.ipynb I do get a bunch of warnings from alchemlyb however
But the resulting free energy estimates still appear reasonable. Note that requesting gromacs=2023.1 will not currently work on osx-arm64 . I will update the tutorials installation docs and report that this has only been tested on linux-64. @chryswoods - when you have a chance could you also rebuild the notebook container with the new environment ? |
Thanks for clarifying about analyse_freenrg. The history of the tool is that it used to be two scripts; analyse_freenrg.py and analyse_freenrg_mbar.py. We merged them into one script using the The port to new pymbar (thanks @fjclark!) doesn't impact the non-mbar code, so shouldn't have any impact on waterswap. There is a test in sire_bigtests, which I'll run. It just verifies that the code can still read the Longer term, we could remove the mbar part of analyse_freenrg with a printout suggesting people use the new tool. Or, we could have analyse_freenrg call the new tool for them (printing out what the command would be, to try to encourage people to migrate). |
Just a note that this still seems to be broken on https://try.openbiosim.org server (exercise 5.1 in https://github.com/OpenBioSim/biosimspace_tutorials/blob/main/04_fep/01_intro_to_alchemy/alchemical_introduction.ipynb) ╭─────────────────────────────── Traceback (most recent call last) ────────────────────────────────╮
│ in <module>:1 │
│ │
│ ❱ 1 pmf_free, overlap_matrix_free = BSS.FreeEnergy.Relative.analyse(f'o_xylene_benzene_for_a │
│ 2 pmf_bound, overlap_matrix_bound = BSS.FreeEnergy.Relative.analyse(f'o_xylene_benzene_for │
│ 3 freenrg_rel = BSS.FreeEnergy.Relative.difference(pmf_bound, pmf_free) │
│ 4 │
│ │
│ /opt/conda/lib/python3.10/site-packages/BioSimSpace/FreeEnergy/_relative.py:462 in analyse │
│ │
│ 459 │ │ │
│ 460 │ │ # SOMD. │
│ 461 │ │ if len(data) > 0: │
│ ❱ 462 │ │ │ return Relative._analyse_somd(work_dir) │
│ 463 │ │ │
│ 464 │ │ # Now check for GROMACS output. │
│ 465 │ │ else: │
│ │
│ /opt/conda/lib/python3.10/site-packages/BioSimSpace/FreeEnergy/_relative.py:636 in _analyse_somd │
│ │
│ 633 │ │ │ stderr=_subprocess.PIPE, │
│ 634 │ │ ) │
│ 635 │ │ if proc.returncode != 0: │
│ ❱ 636 │ │ │ raise _AnalysisError("SOMD free-energy analysis failed!") │
│ 637 │ │ │
│ 638 │ │ # Re-run without subsampling if the subsampling has resulted in less than 50 sam │
│ 639 │ │ with open("%s/mbar.txt" % work_dir) as file: │
╰──────────────────────────────────────────────────────────────────────────────────────────────────╯
AnalysisError: SOMD free-energy analysis failed!
|
Thanks for the update. I assume that the fixed code hasn't yet made it's way to the server. Hopefully this will be resolved when the server image is rebuilt. |
Are we planning on rebuilding? The server currently has both sire and BioSimSpace 2023.3.0 installed. I hadn't upgraded anything as I assumed that everything was working. Do you want us to update the server to the latest stable sire and BioSimSpace versions (2023.3.2 and 2023.3.1 respectively)? I can do that tonight if this is desired. @jmichel80 - what do you want? |
Yes can you rebuild ? |
All updated - the notebook server should be identical to before, except that sire 2023.3.2 and BioSimSpace 2023.3.1 are now installed |
This is because this PR was never backported into |
Is it easier to just change the version of |
Ah yes - I remember now that that this PR changes the dependencies and features of sire, so is saved for a major release (2023.4.0) not a point release. |
Since it is a change in a single file, I could monkey-patch the notebook server to have the new version of |
Ah yes, good point, this will change the dependency resolution so probably best to save for the next release. As you say, it's a single file patch (just do a patch in the docker build) or simply pin to the old version of pymbar, which still allows the required version of cinnabar to be resolved. |
Ok, it is patched and updated. Your Jupyter kernels will need to restart (I think you have had them interrupted). I've just updated |
This now looks like it is working
|
The free energy analysis seems to be working, but the LOMAP network generation in 01_setup_RBFE notebook (https://github.com/OpenBioSim/biosimspace_tutorials/blob/main/04_fep/02_RBFE/01_setup_rbfe.ipynb) (Section 2.2) seems to be broken now: |
I you add
Here it looks like the In my local environment I have: mamba list pydot feature_runner ✱ ◼
# packages in environment at /home/lester/.conda/envs/openbiosim:
#
# Name Version Build Channel
pydot 1.4.2 py310hff52083_3 conda-forge On the server I see: mamba list pydot
# packages in environment at /opt/conda:
#
# Name Version Build Channel
pydot 1.2.4 py_0 conda-forge I'll try updating the version to see if it works. It's weird that a more recent build has resolved an older version of the package. Hopefully this isn't a general problem, which might mean that we need to add a pin to the BIoSImSpace recipe. |
Yes, simply doing the following in the terminal fixes things. (Not sure if it breaks anything else, though.) mamba install -c conda-forge pydot=1.4.2 |
I've just checked and |
I'm uploading an image with pydot 1.4.2 installed... |
This seems to work now, but the debug messages get printed to the output of the first cell (where the first debug message about RDKit gets printed on import). It is a bit confusion, but the output is right. |
Yes, I noticed that too. I hadn't seen that crazy long |
I've attempted to test the ABFE notebooks many times now, and each time it stops with the same error: I have been running all cells initially, during which I haven't yet seen this error on either notebook. So far I've only hit this when running the exercises after the initial run through, but sometimes on cells which I wouldn't expect to be computationally intense, e.g. reading the restraint correction from the restraint. |
This is most likely an issue relating to running out of memory. While the notebook is limited to 4GB per user, there is only 8 GB of memory on the server. This means that two people using it at the same time can exhaust the memory on the server, and crash each other's kernels. It is worth clearing / closing one notebook before starting another, to minimise memory usage. It may also be worth coordinating to ensure that only one person today is going through and testing everything (or you take turns) |
I have tested all FEP notebooks (except ABFE) and everything seems to work now, thanks for your help! |
(on the day the server will have 256 GB so should be able to support ~60 simultaneous users) |
I've deleted your pod @akalpokas so now only @fjclark's is running on the server. Assuming no-one starts another pod, there should now be enough memory (assuming it fits under 4GB) |
I've created the branch here. However, it might be good to keep the exercises for users running on their own machines. If that sounds reasonable, could someone please create a new branch for me to create a PR to (I don't seem to have permission)? Otherwise, I'll reopen my PR to devel. Thanks. |
I think 4GB is about what you could expect from people's laptops. The spread will likely be 4-16GB, with 8GB being the norm. So it may be worth keeping them as examples and not having people run them locally, as out of memory on a laptop will cause more issues than on the server. Best case will be swapping, but worst case will be slowing down the laptops a lot or randomly crashing things. |
Ok, thanks for the comment. I've reopened the PR. |
closing due to inactivity |
Runtime error for exercise 4.5
Manually running analyse_freenrg on the somd outputs used for the exercise gives
I suspect the problem is that pymbar 4 is used in the environment
Is the solution to build the server image against pymbar3.1.1 (https://anaconda.org/conda-forge/pymbar/files) and also update the installation instructions at https://github.com/OpenBioSim/biosimspace_tutorials#readme ?
The text was updated successfully, but these errors were encountered: