Skip to content
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

Add support for openff-nagl for generation of partial charges #267

Merged
merged 15 commits into from
Apr 9, 2024

Conversation

lohedges
Copy link
Contributor

This PR adds support for using openff-nagl to generate partial charges during parameterisation. Rather than adding openff-nagl as a hard dependency, I decided to check for availability at runtime and selectively use it if available. The user can choose to disable NAGL when calling an OpenFF parameterisation function using use_nagl=False, in which case it will use AmberTools. To activate the feature, a user should only need to install openff-nagl-base and openff-nagl-models on top of the base BioSimSpace environment. Thankfully these are pinned against specific versions of openff-toolkit, so we should always be able to resolve a compatible environment. (The full openff-nagl package pulls in over 2GB of dependencies, so it is desirable to exclude it if it's not absolutely necessary.)

  • 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 added a test for any new functionality in this pull request: [y]
  • I confirm that I have added documentation (e.g. a new tutorial page or detailed guide) for any new functionality in this pull request: [y]
  • I confirm that I have permission to release this code under the GPL3 license: [y]

Suggested reviewers:

@chryswoods

@lohedges lohedges added enhancement New feature or request exscientia Related to work with Exscientia labels Mar 25, 2024
@lohedges lohedges requested a review from chryswoods March 25, 2024 16:08
@lohedges
Copy link
Contributor Author

All CI has started failing with:

Run conda mambabuild -c conda-forge -c openbiosim/label/dev /home/runner/work/biosimspace/biosimspace/biosimspace/recipes/biosimspace
Traceback (most recent call last):
  File "/home/runner/miniconda3/envs/bss_build/bin/conda-mambabuild", line 6, in <module>
    from boa.cli.mambabuild import main
  File "/home/runner/miniconda3/envs/bss_build/lib/python3.9/site-packages/boa/cli/mambabuild.py", line 21, in <module>
    from boa.core.solver import MambaSolver
  File "/home/runner/miniconda3/envs/bss_build/lib/python3.9/site-packages/boa/core/solver.py", line 7, in <module>
    from boltons.setutils import IndexedSet
ModuleNotFoundError: No module named 'boltons'

Looks like boa is missing a dependency. Bah.

@lohedges
Copy link
Contributor Author

Okay, manually adding boltons to our CI dependencies didn't fix things:

Traceback (most recent call last):
  File "/home/runner/miniconda3/envs/bss_build/bin/conda-mambabuild", line 10, in <module>
    sys.exit(main())
  File "/home/runner/miniconda3/envs/bss_build/lib/python3.9/site-packages/boa/cli/mambabuild.py", line 301, in main
    call_conda_build(action, config)
  File "/home/runner/miniconda3/envs/bss_build/lib/python3.9/site-packages/boa/cli/mambabuild.py", line 273, in call_conda_build
    result = api.build(
  File "/home/runner/miniconda3/envs/bss_build/lib/python3.9/site-packages/conda_build/api.py", line 253, in build
    return build_tree(
  File "/home/runner/miniconda3/envs/bss_build/lib/python3.9/site-packages/conda_build/build.py", line 3799, in build_tree
    packages_from_this = build(
  File "/home/runner/miniconda3/envs/bss_build/lib/python3.9/site-packages/conda_build/build.py", line 2469, in build
    output_metas = expand_outputs([(m, need_source_download, need_reparse_in_env)])
  File "/home/runner/miniconda3/envs/bss_build/lib/python3.9/site-packages/conda_build/render.py", line 923, in expand_outputs
    for output_dict, m in deepcopy(_m).get_output_metadata_set(
  File "/home/runner/miniconda3/envs/bss_build/lib/python3.9/site-packages/conda_build/metadata.py", line 2565, in get_output_metadata_set
    conda_packages = finalize_outputs_pass(
  File "/home/runner/miniconda3/envs/bss_build/lib/python3.9/site-packages/conda_build/metadata.py", line 909, in finalize_outputs_pass
    fm = finalize_metadata(
  File "/home/runner/miniconda3/envs/bss_build/lib/python3.9/site-packages/conda_build/render.py", line 637, in finalize_metadata
    build_unsat, host_unsat = add_upstream_pins(
  File "/home/runner/miniconda3/envs/bss_build/lib/python3.9/site-packages/conda_build/render.py", line 452, in add_upstream_pins
    build_deps, build_unsat, extra_run_specs_from_build = _read_upstream_pin_files(
  File "/home/runner/miniconda3/envs/bss_build/lib/python3.9/site-packages/conda_build/render.py", line 431, in _read_upstream_pin_files
    deps, actions, unsat = get_env_dependencies(
  File "/home/runner/miniconda3/envs/bss_build/lib/python3.9/site-packages/conda_build/render.py", line 151, in get_env_dependencies
    actions = environ.get_install_actions(
  File "/home/runner/miniconda3/envs/bss_build/lib/python3.9/site-packages/boa/cli/mambabuild.py", line 180, in mamba_get_install_actions
    link_precs = mamba_get_package_records(
  File "/home/runner/miniconda3/envs/bss_build/lib/python3.9/site-packages/boa/cli/mambabuild.py", line 142, in mamba_get_package_records
    _, link_precs = solver.solve_for_unlink_link_precs(_specs, prefix)
  File "/home/runner/miniconda3/envs/bss_build/lib/python3.9/site-packages/boa/core/solver.py", line 252, in solve_for_unlink_link_precs
    return to_unlink_link_precs(
  File "/home/runner/miniconda3/envs/bss_build/lib/python3.9/site-packages/boa/core/solver.py", line 87, in to_unlink_link_precs
    unlink_precs, link_precs = diff_for_unlink_link_precs(
  File "/home/runner/miniconda3/envs/bss_build/lib/python3.9/site-packages/conda/core/solve.py", line 1152, in diff_for_unlink_link_precs
    assert isinstance(final_precs, IndexedSet)
AssertionError

Presumably boa is broken, or incompatible with the version of conda / conda-bulild. I'll leave until tomorrow. (Fingers crossed it will be magically fixed overnight.)

@lohedges
Copy link
Contributor Author

It's a sad day when the solution to conda hell is now broken.

@lohedges lohedges temporarily deployed to biosimspace-build March 28, 2024 12:11 — with GitHub Actions Inactive
@lohedges lohedges temporarily deployed to biosimspace-build March 28, 2024 12:11 — with GitHub Actions Inactive
@lohedges lohedges temporarily deployed to biosimspace-build March 28, 2024 12:11 — with GitHub Actions Inactive
@lohedges lohedges temporarily deployed to biosimspace-build March 28, 2024 13:05 — with GitHub Actions Inactive
@lohedges lohedges temporarily deployed to biosimspace-build March 28, 2024 13:05 — with GitHub Actions Inactive
@lohedges lohedges temporarily deployed to biosimspace-build March 28, 2024 13:05 — with GitHub Actions Inactive
@lohedges lohedges temporarily deployed to biosimspace-build March 28, 2024 13:05 — with GitHub Actions Inactive
@lohedges lohedges temporarily deployed to biosimspace-build March 28, 2024 13:05 — with GitHub Actions Inactive
chryswoods
chryswoods previously approved these changes Mar 28, 2024
Copy link
Contributor

@chryswoods chryswoods left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good. I like that we don't depend on nagl_models being installed, and that we check at runtime. This is a pattern I am increasingly using in sire. R has a concept of "suggests" with run-time checking for dependencies which is similar.

Good spot that the miniconda-setup recipe has gone up a version. Thanks to this PR I've now bumped the version in sire.

@lohedges lohedges dismissed chryswoods’s stale review April 8, 2024 08:58

The merge-base changed after approval.

@lohedges lohedges temporarily deployed to biosimspace-build April 8, 2024 09:24 — with GitHub Actions Inactive
@lohedges lohedges temporarily deployed to biosimspace-build April 8, 2024 09:24 — with GitHub Actions Inactive
@lohedges lohedges temporarily deployed to biosimspace-build April 8, 2024 09:24 — with GitHub Actions Inactive
@lohedges lohedges temporarily deployed to biosimspace-build April 8, 2024 09:24 — with GitHub Actions Inactive
@lohedges lohedges temporarily deployed to biosimspace-build April 8, 2024 09:24 — with GitHub Actions Inactive
@lohedges lohedges merged commit 27739b6 into devel Apr 9, 2024
5 checks passed
@lohedges lohedges deleted the feature_nagl branch April 9, 2024 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request exscientia Related to work with Exscientia
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants