Skip to content

Add ofeb version - #179

Open
jaclark5 wants to merge 5 commits into
mainfrom
add_ofeb_version
Open

jaclark5 wants to merge 5 commits into
mainfrom
add_ofeb_version

Conversation

@jaclark5

@jaclark5 jaclark5 commented Nov 5, 2026

Copy link
Copy Markdown
Collaborator

Benchmark Results Submission

Description

Add openfe-benchmarks version to the submission.yaml to record the version of a network used to generate results.

Change level:

See RELEASE.md.

  • major: changes existing data
  • minor: additive
  • bug: fix, script, docs, new results

Submission

  • Labeled PR with version level major, minor, or bug
  • [NA] submission.yaml made with openfe_benchmarks/scripts/prepare_metadata_submission.py
  • [NA] computational_results.json.bz2 made with openfe_benchmarks/scripts/generate_results_archives.py
  • [NA] Single experiment; settings differing from OpenFE defaults described in summary of this PR submission
  • CHANGELOG.md updated under Unreleased

Checked by CI

  • submission.yaml is valid and loads
  • submission_id matches directory name and is unique
  • Results file exists at the path in results

Notes

@jaclark5
jaclark5 requested review from jthorton and a balanced review from Copilot November 5, 2026 17:05

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.

Note

Copilot was unable to complete this review at the selected tier due to model policy or plan restrictions.

🟡 Changes recommended

Several moderate-severity issues remain that can break backward compatibility and runtime behavior (unconditional pop causing KeyError, import-time importlib.metadata.version() failures in example scripts, and a mismatched metadata key that prevents correct Zenodo description rendering), so the changes need fixes before approval.

6 open issues
What changed in this PR

This pull request adds openfe_benchmarks_version to benchmark submission metadata so downstream consumers can record which openfe-benchmarks version was used to generate the network(s) behind a results submission.

Changes:

  • Extend metadata preparation/output to include openfe_benchmarks_version in submission.yaml (and related description text).
  • Update example planning scripts to annotate planned transformations with openfe_benchmarks_version.
  • Backfill existing archived results/**/submission.yaml files to include the new field, and update results loading/export to handle it.

Unresolved review items to address before approval:

  • Backward compatibility: _extract_benchmark_fields unconditionally pops openfe_benchmarks_version and should use a default to avoid KeyError on older submissions/internal callers.
  • Examples robustness: importlib.metadata.version() can raise when running examples from a source checkout; add a fallback so examples still run.
  • Key typo: Zenodo description path uses openffe_benchmarks_version instead of openfe_benchmarks_version, causing the description to remain TODO even when populated.

This pull request requires changes.

File Summary
openfe_benchmarks/​scripts/​prepare_metadata_submission.py Collects/writes openfe_benchmarks_version into generated submission metadata; fix mismatched openffe_benchmarks_version key in Zenodo description generation.
openfe_benchmarks/​scripts/​_example_plan_septop.py Annotates generated transformations with openfe_benchmarks_version; add fallback for importlib.metadata.version().
openfe_benchmarks/​scripts/​_example_plan_rbfe.py Annotates generated transformations with openfe_benchmarks_version; add fallback for importlib.metadata.version().
openfe_benchmarks/​scripts/​_example_plan_rbfe_pontibus.py Annotates generated transformations with openfe_benchmarks_version; add fallback for importlib.metadata.version().
openfe_benchmarks/​scripts/​_example_plan_asfe.py Annotates generated transformations with openfe_benchmarks_version; add fallback for importlib.metadata.version().
openfe_benchmarks/​results/​2026-09-28-openff3.0.0-alpha2b_opc3-jacs/​submission.yaml Backfills openfe_benchmarks_version in archived submission metadata.
openfe_benchmarks/​results/​2026-09-03_openff-2.3.0_opc3_jacs_set/​submission.yaml Backfills openfe_benchmarks_version in archived submission metadata.
openfe_benchmarks/​results/​2026-08-25_ff14sb_openff-2.1.1_jacs/​submission.yaml Backfills openfe_benchmarks_version in archived submission metadata.
openfe_benchmarks/​results/​2026-08-06-openff-2.3.0-solvation_set_freesolv/​submission.yaml Backfills openfe_benchmarks_version in archived submission metadata.
openfe_benchmarks/​results/​2026-08-05-openff3.0.0-alpha1b_tip3p-jacs/​submission.yaml Backfills openfe_benchmarks_version in archived submission metadata.
openfe_benchmarks/​results/​2026-08-05_openff-3.0.0-alpha0_opc3_jacs/​submission.yaml Backfills openfe_benchmarks_version in archived submission metadata.
openfe_benchmarks/​results/​2026-08-05_ff14sb_openff-3.0.0-alpha1b_tip3p_jacs_tyk2_thrombin/​submission.yaml Backfills openfe_benchmarks_version in archived submission metadata.
openfe_benchmarks/​results/​2026-08-04-openff3.0.0-alpha1b_opc3-jacs/​submission.yaml Backfills openfe_benchmarks_version in archived submission metadata.
openfe_benchmarks/​results/​2026-07-31_sage_230_all/​submission.yaml Backfills openfe_benchmarks_version in archived submission metadata.
openfe_benchmarks/​results/​2026-06-22-tyk2-alchemicalarchive-test/​submission.yaml Backfills openfe_benchmarks_version in archived submission metadata.
openfe_benchmarks/​results/​2026-03-18-openmm-840-qa-testing/​submission.yaml Backfills openfe_benchmarks_version in archived submission metadata.
openfe_benchmarks/​results/​2026-02-12_sage_230_jacs_set/​submission.yaml Backfills openfe_benchmarks_version in archived submission metadata.
openfe_benchmarks/​results/​_benchmark_results.py Updates submission schema/loader/export to include openfe_benchmarks_version; avoid KeyError by not unconditionally popping the field.

Review effort: Balanced

license=yaml_fields.pop("license"),
openfe_version=yaml_fields.pop("openfe_version"),
openmm_version=yaml_fields.pop("openmm_version"),
openfe_benchmarks_version=yaml_fields.pop("openfe_benchmarks_version"),

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

[ignore comment] The older submission.yaml files had this field added, and we require that it always be defined so we would want it to error.

FILENAME = f"network_{SYSTEM_GROUP}_{SYSTEM_NAME}_asfe.json"
WATER_MODEL = "tip3p" # or opc
SUBSET = "subset_openff_filtered"
OPENFE_BENCHMARKS_VERSION = version("openfe_benchmarks")

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

[ignore comment] before this line there are modules imported from openfe-benchmarks. If there were to be an error from openfe-benchmarks not being installed, it would happen there not here.

f"alchemical_network_{SYSTEM_GROUP}_{SYSTEM_NAME}_nacl.json"
)
OUTPUT_DIR = "outputs"
OPENFE_BENCHMARKS_VERSION = version("openfe_benchmarks")

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

[ignore comment] before this line there are modules imported from openfe-benchmarks. If there were to be an error from openfe-benchmarks not being installed, it would happen there not here.

f"alchemical_network_{SYSTEM_GROUP}_{SYSTEM_NAME}_nacl.json"
)
OUTPUT_DIR = "outputs"
OPENFE_BENCHMARKS_VERSION = version("openfe_benchmarks")

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

[ignore comment] before this line there are modules imported from openfe-benchmarks. If there were to be an error from openfe-benchmarks not being installed, it would happen there not here.

f"septop_alchemical_network_{SYSTEM_GROUP}_{SYSTEM_NAME}_nacl.json"
)
OUTPUT_DIR = "outputs"
OPENFE_BENCHMARKS_VERSION = version("openfe_benchmarks")

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

[ignore comment] before this line there are modules imported from openfe-benchmarks. If there were to be an error from openfe-benchmarks not being installed, it would happen there not here.

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.

2 participants