Skip to content

Asfe freesolv - #134

Open
jaclark5 wants to merge 17 commits into
mainfrom
asfe_freesolv
Open

Asfe freesolv#134
jaclark5 wants to merge 17 commits into
mainfrom
asfe_freesolv

Conversation

@jaclark5

@jaclark5 jaclark5 commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

** Do not merge until after #132 **

This PR completes our first ASFE submission. Luckily the _no_test_example_rbfe_submission.py worked perfectly, so renamed to _no_test_example_rbfe_asfe_submission.py

Changes include:

  • rename _no_test_example_rbfe_submission.py to _no_test_example_rbfe_asfe_submission.py
  • Add "calculation_type" metadata field to prepare_metadata_submission.py to auto populate "rbfe" vs "asfe", otherwise the only metadata field with this distinction is an optional tag.
    • Added "calculation_type" metadata field to existing submission.yaml files
  • Tags now automatically get asfe/rbfe, openfe/pontibus, and alchemicalarchive/alchemicalnetwork, eliminating human error in optional tags
  • computational_results.json.bz2 for ASFEs now aligns with keys used in dgs of RBFEs
  • New openfe_benchmarks/scripts/_no_test_example_plot_asfe.py. This cannot be tested because I cannot install cinnabar==0.6.1 with pontibus==0.4.0 and openfe<1.9. I couldn't get this plotting to run with cinnabar==0.5.0. I believe this will be resolved when alchemiscale is updated.

@jaclark5
jaclark5 requested review from jthorton and a lite review from Copilot August 7, 2026 15:07

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR finalizes an initial ASFE (absolute solvation free energy) submission workflow by extending the metadata-generation pipeline to better distinguish ASFE vs RBFE submissions, standardizing submission tags to reduce manual errors, and adding utilities/examples for plotting ASFE results.

Changes:

  • Updated metadata generation to be more robust to missing mapping annotations and to emit a new calculation_type field in submission.yaml.
  • Automated/enhanced tag generation (calculation mode, network mode, and protocol libraries) and updated default tags usage in example scripts.
  • Added ASFE result processing/plotting support (build_femap_from_absolute_results, _no_test_example_plot_asfe.py) and added/updated committed submission YAMLs.

Reviewed changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
openfe_benchmarks/scripts/prepare_metadata_submission.py Adds ASFE-aware metadata behavior (calculation_type, tag enrichment) and handles missing trans.mapping.
openfe_benchmarks/scripts/_results_utils.py Adds absolute-results FEMap builder for ASFE-style outputs and experimental-data augmentation.
openfe_benchmarks/scripts/_no_test_example_rbfe_asfe_submission.py Updates example defaults to rely on auto-tagging; notes ASFE compatibility.
openfe_benchmarks/scripts/_no_test_example_plot_asfe.py New example script to plot ASFE DGs vs experiment (requires newer cinnabar).
openfe_benchmarks/scripts/_no_test_example_mutlinetwork_rbfe_submission.py Makes multinetwork example more generic and relies on auto-tagging.
openfe_benchmarks/results/2026-08-06-openff-2.3.0-solvation_set_freesolv/submission.yaml New ASFE submission YAML including calculation_type.
openfe_benchmarks/results/2026-08_04-openff3.0.0-alpha1b_opc3-jacs/submission.yaml Adds calculation_type to an RBFE submission YAML.
openfe_benchmarks/results/2026-06-22-tyk2-alchemicalarchive-test/submission.yaml Adds calculation_type to an RBFE submission YAML.
openfe_benchmarks/results/2026-03-18-openmm-840-qa-testing/submission.yaml Adds calculation_type to an RBFE submission YAML.
openfe_benchmarks/results/2026-02-12_sage_230_jacs/submission.yaml Adds calculation_type to an RBFE submission YAML.
devtools/conda-envs/environment.yml Renames the conda environment and notes cinnabar constraint.
Suppressed comments (1)

openfe_benchmarks/scripts/prepare_metadata_submission.py:1766

  • metadata.calculation_mode is a string, so comparing it to a list will always be false. This makes small_molecule_forcefield_yaml always blank, even for RBFE/SepTop submissions.
    if metadata.calculation_mode == ["rbfe", "septop"]:

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread openfe_benchmarks/scripts/prepare_metadata_submission.py Outdated
Comment thread devtools/conda-envs/environment.yml Outdated
Comment on lines +134 to +137
if solutes_no_uncertainty:
raise ValueError(
f"Not all solutes have dg_uncertainty for {system_group} {system_name}: {solutes_no_uncertainty}"
)
Comment thread openfe_benchmarks/scripts/_results_utils.py Outdated
Comment thread openfe_benchmarks/scripts/prepare_metadata_submission.py
jaclark5 and others added 2 commits August 7, 2026 11:17
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Base automatically changed from submit_alpha1b_opc3 to main August 10, 2026 15:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 15 out of 16 changed files in this pull request and generated 1 comment.

Suppressed comments (3)

openfe_benchmarks/scripts/_results_utils.py:156

  • The experimental data file is opened without a context manager, which can leak file descriptors in long-running workflows. Use a with open(...) block when loading JSON.
        experimental_file = benchmark_data.reference_data[
            "experimental_solvation_free_energy_data"
        ]
        experimental_data = json.load(open(experimental_file), cls=JSON_HANDLER.decoder)
        n_experimental_points = 0

openfe_benchmarks/scripts/_no_test_example_plot_asfe.py:22

  • Compression detection should use the path suffix rather than substring matching ("bz2" can appear elsewhere in the path). This makes loading behavior deterministic.
    open_func = bz2.open if "bz2" in results_file else open

openfe_benchmarks/results/2026-08-06-openff-2.3.0-solvation_set_freesolv/submission.yaml:11

  • Grammar: the summary says "1 unique solvents" but should be singular when the count is 1.
  cofactors. The archive contains 603 edges across 603 unique solutes and 1 unique solvents. Results

Comment thread openfe_benchmarks/scripts/generate_results_archives.py

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 15 out of 16 changed files in this pull request and generated no new comments.

Suppressed comments (1)

openfe_benchmarks/scripts/_results_utils.py:145

  • This comprehension accesses result["ligand"] unconditionally, but the surrounding logic in this function is explicitly written to support both the new key layout (ligand + dg/dg_uncertainty) and the legacy layout (solute + estimate/estimate_error): the NaN check falls back to estimate_error, the value extraction below falls back to estimate, and _asfe_result_key falls back to solute. For a legacy-format record (the exact case the estimate_error branch is meant to handle), result["ligand"] will raise KeyError before that fallback is reached. Use the same solute-resolution helper here so the backward-compatibility handling is consistent.
            result["ligand"]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants