Create freesolv and mnsol subsets - #118
Conversation
There was a problem hiding this comment.
Pull request overview
Defines repeatable OpenFF-focused subsets for FreeSolv and MNSol solvation benchmarks, and updates FreeSolv experimental data generation to use InChIKeys derived from the distributed 3D SDF structures.
Changes:
- Add
subset_datadiscovery/loading to benchmark system metadata and validate subset JSONs in tests. - Introduce data-generation scripts to produce
subset_openff_filteredandsubset_openff_smallfor FreeSolv and MNSol. - Update FreeSolv experimental JSON generation to compute (RDKit) InChI/InChIKey from
ligands.sdf, and update stored experimental identifiers accordingly.
Reviewed changes
Copilot reviewed 10 out of 13 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
| openfe_benchmarks/data/_benchmark_systems.py | Adds subset*.json discovery and exposes subset_data on BenchmarkData. |
| openfe_benchmarks/tests/test_benchmark_data.py | Adds subset JSON load + basic integrity checks. |
| openfe_benchmarks/scripts/_example_plan_asfe.py | Example ASFE planning script now filters experimental entries by a named subset. |
| openfe_benchmarks/data/data_generation/generate_freesolv_exp_data.py | Regenerates experimental FreeSolv JSON using InChIKeys derived from shipped SDF structures. |
| openfe_benchmarks/data/data_generation/define_mnsol_openff_subset.py | New script to filter/downselect MNSol into OpenFF benchmarking subsets. |
| openfe_benchmarks/data/data_generation/define_freesolv_openff_subset.py | New script to filter/downselect FreeSolv into OpenFF benchmarking subsets. |
| openfe_benchmarks/data/benchmark_systems/solvation_set/mnsol_neutral/PREPARATION_DETAILS.md | Documents new subset files and filtering/downselection criteria for MNSol. |
| openfe_benchmarks/data/benchmark_systems/solvation_set/freesolv/PREPARATION_DETAILS.md | Documents new subset files and filtering/downselection criteria for FreeSolv. |
| openfe_benchmarks/data/benchmark_systems/solvation_set/freesolv/subset_openff_small.json | Adds the down-selected “small” FreeSolv subset definition. |
| openfe_benchmarks/data/benchmark_systems/solvation_set/freesolv/experimental_solvation_free_energy_data.json | Updates stored solute InChI/InChIKey values to match SDF-derived identifiers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
I'm also expecting feedback from @lilyminium on how the down selection takes place, since these subsets won't likely be changed once we start running. |
hannahbaumann
left a comment
There was a problem hiding this comment.
Thanks @jaclark5 ! I've only done a partial review for now and haven't made it through the large define_freesolv_mnsol_openff_subsets.py file, will do that later.
|
|
||
| ## Subsets | ||
|
|
||
| Regenerate all subsets: run `run.sh` in `data_generation/`. |
There was a problem hiding this comment.
I don't think the run.sh is currently present in that directory.
| ## Charging Solutes / Solvents | ||
| ## Partial Charges | ||
|
|
||
| - Licensing prevented including MNSol experimental values. After properly obtaining the dataset, a user can locally generate the needed experimental data file with the [generate_mnsol_data.py](../../../data_generation/generate_mnsol_data.py) script. |
There was a problem hiding this comment.
This may need a separate heading since it doesn't specify anything partial charge related.
There was a problem hiding this comment.
Good point, I moved it
|
|
||
| - Licensing prevented including MNSol experimental values. After properly obtaining the dataset, a user can locally generate the needed experimental data file with the [generate_mnsol_data.py](../../../data_generation/generate_mnsol_data.py) script. | ||
| - Experimental uncertainties are set to 0.2 kcal/mol for all neutral entries, following the recommendation in the MNSol documentation. No newline at end of file | ||
| Regenerate all subsets: run `run.sh` in `data_generation/`. |
There was a problem hiding this comment.
As above, file not present.
| Regenerate all subsets: run `run.sh` in `data_generation/`. | ||
|
|
||
| ### subset_openff_filtered.json | ||
| 1587 systems retained from ~3037 MNSol database entries, covering 232 unique solutes across multiple solvent environments. Excluded: 219 charged systems; 528 entries with solvents on the skip list; 313 entries with solvents bearing disqualifying SMIRKS; 198 with missing solvent SMILES; 44 with solutes containing out-of-scope elements; 42 self-solvation entries (solute = solvent); 40 with solvents containing out-of-scope elements; 24 with solutes on the skip list; 18 with undefined solvent stereochemistry; 10 with disqualifying solute SMIRKS; 10 with undefined solute stereochemistry; and 4 with missing solute SMILES. Entries may satisfy multiple exclusion criteria; totals are not additive. |
There was a problem hiding this comment.
maybe add a brief explanation what the skip list is.
| "mnsol-0019": { | ||
| "mnsol No.": "0019", | ||
| "solute_name": "4-methyl-2-pentanone", | ||
| "solvent_name": "4methyl2pentanone", |
There was a problem hiding this comment.
This means the subset small does include self-solvation entries?
There was a problem hiding this comment.
0.0 ... good catch, I just when through the MNSol dataset again and standardized names and smiles to prevent duplicates.
| elif solvent_inchikey not in mol_dict: | ||
| logger.warning( | ||
| f"Solvent '{solvent_inchikey}' not found in SDF; skipping network '{network_name}'" | ||
| f"Solvent '{solvent_inchikey}' not found in SDF; skipping network '{transformation_name}'" |
There was a problem hiding this comment.
| f"Solvent '{solvent_inchikey}' not found in SDF; skipping network '{transformation_name}'" | |
| f"Solvent '{solvent_inchikey}' not found in SDF; skipping transformation '{transformation_name}'" |
hannahbaumann
left a comment
There was a problem hiding this comment.
Thanks @jaclark5 , lgtm!
Not all of the freesolv and mnsol datasets fit the needs of OpenFF benchmarking (e.g., outside of element coverage) using OpenFE (e.g, charged). This PR defines common subsets to be used in benchmarking in a repeatable way. For both FreeSolv and MNSol there are two subsets:
This PR also updates:
generate_freesolv_exp_data.py:generate_freesolv_exp_data.pyThe
ligands_*.sdffiles will be updated in a different PR where I'll check the possibility of accidentally adding hydrogenDo not review
define_freesolv_mnsol_openff_subsets.pythis file has been reviewed by our science team. If you really want to take a look I'm open to suggestions, but since this defines OpenFF subsets I don't think it's the OpenFE team's responsibility