Skip to content

Add results from the openmm840 qa#117

Open
jthorton wants to merge 19 commits into
mainfrom
openmm840_qa
Open

Add results from the openmm840 qa#117
jthorton wants to merge 19 commits into
mainfrom
openmm840_qa

Conversation

@jthorton

Copy link
Copy Markdown
Contributor

A draft of the results object from a local qa run of openmm-8.4.0.

@IAlibay IAlibay left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Couple of small thoughts.

Comment thread openfe_benchmarks/results/2026-03-18-openmm-840-qa-testing/submission.yaml Outdated
date: 2026-03-18

# REQUIRED: OpenFE version used to produce the gathered reports
openfe_version: 1.8.0

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Am I correct in remembering that we would put the full environment file on zenodo? Would it still make sense to have a few key entries here like openmm version and the openff toolkit version?

# REQUIRED: license for the submission (e.g. CC-BY-4.0)
license: CC-BY-4.0

# RECOMMENDED / OPTIONAL metadata

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we add extra experiment data here? Like the FEP method used, simulation length, number of repeats, etc...

@jthorton

Copy link
Copy Markdown
Contributor Author

TODO:

  • upload the raw results to zenodo
  • make the submission yaml again using the script and add the link the the zenodo entry
  • ask for review

@jaclark5 jaclark5 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Here are some suggested changes to be consistent between the submission.yaml and the prepare_metadata_submission.py

Comment thread openfe_benchmarks/scripts/_example_generate_results_local.py Outdated
Comment thread openfe_benchmarks/results/2026-03-18-openmm-840-qa-testing/submission.yaml Outdated
Comment thread openfe_benchmarks/results/2026-03-18-openmm-840-qa-testing/submission.yaml Outdated
@jthorton

jthorton commented Jun 3, 2026

Copy link
Copy Markdown
Contributor Author

Update _example_plan_rbfe.py to make sure that the "system_name" and "system_group" are saved in the mapping annotations.

@jaclark5

jaclark5 commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator

I think I updated everything according to the meeting today.
@jthorton you'll have to:

  • rerun the results file generation
  • create zenodo record, here's an initial draft of the description output by the script
    zenodo_description.md

@jaclark5

jaclark5 commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator

Update _example_plan_rbfe.py to make sure that the "system_name" and "system_group" are saved in the mapping annotations.

This should be resolved

Comment on lines +162 to +167

qualname = payload.get("__qualname__")
if qualname == "AlchemicalArchive":
archive_obj = payload
elif qualname == "AlchemicalNetwork":
network_obj = payload

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This script looks great, but how do we feel about working with the keychain dicts? would it be better to deseralise the objects and use the API to interagate them when extracting the information that would be my personal preference.

# RECOMMENDED / OPTIONAL metadata for protocol settings
protocol_settings:
- protocol: RelativeHybridTopologyProtocol
count: 3

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

How should this work when a network contains a mix of protocol settings will the networks be listed under both protocols?

Comment on lines +337 to +342
names = [trans.name for trans in _transformation_refs(network_obj, mode)]
if names:
# !!!! NoteHere !!! what is hard coded to detect calculation type?
if any(n.startswith("complex_") or n.startswith("solvent_") for n in names):
return "rbfe"
return "asfe"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Checking the protocol type on the transformation would be safer.

openmm_version: TODO
openff_toolkit_version: TODO
mapper: "KartografAtomMapper 1.2.0 (LSA)"
forcefield: ["ff14SB", "phosaa10", "tip3p_HFE_multivalent", "tip3p_standard"]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is listed here and in the protocol settings is this the searchable tag? If so we probably want the small molecule forcefield here as well.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Or is the tags list intended to be the searchable keys?

This submission describes the charge_annihilation_set, jacs_set RBFE benchmark
(charge_annihilation_set: egfr, irak4_s2, irak4_s3; jacs_set: p38, tyk2) prepared with
tip3p_HFE_multivalent/ff14SB/phosaa10/tip3p_standard and am1bcc_at. The submission contains 160
edges, 45 unique ligands, and 1 unique proteins. Note this means the charge annihilation sets are

@jthorton jthorton Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

1 unique proteins

This is wrong

summary: |
This submission describes the charge_annihilation_set, jacs_set RBFE benchmark
(charge_annihilation_set: egfr, irak4_s2, irak4_s3; jacs_set: p38, tyk2) prepared with
tip3p_HFE_multivalent/ff14SB/phosaa10/tip3p_standard and am1bcc_at. The submission contains 160

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Force fields are listed again but not the small molecule force field or charges.

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