Tyk2 alchemicalarchive example - #127
Conversation
| tmp_path = tmp.name | ||
|
|
||
| try: | ||
| archive = AlchemicalArchive.from_json(tmp_path) |
There was a problem hiding this comment.
Can pass the content directly to this method https://gufe.openfree.energy/en/latest/generated/gufe.tokenization.html#gufe.tokenization.GufeTokenizable.from_json
|
|
||
| if "unit_estimate" in outputs: | ||
| estimate_qty = outputs["unit_estimate"] | ||
| estimate = estimate_qty.magnitude |
There was a problem hiding this comment.
use consistent units for the analysis and do m_as
|
|
||
| # Extract estimate from the DAG result | ||
| if dag_results_list: | ||
| dag_result = dag_results_list[0] |
There was a problem hiding this comment.
We need to use the repeat results if they are present
|
|
||
| @click.command() | ||
| @click.option( | ||
| "--archive", |
There was a problem hiding this comment.
How do we want to handle results with many archives, the archive generation script can produce multiple in a single run https://github.com/OpenFreeEnergy/openfe-benchmarks/blob/main/openfe_benchmarks/scripts/_tmp_alchemiscale_gather.py. Is the user expected to then combine the computational_results.json files after or should we adapt this script to accept multiple inputs.
| complex_dg_uncertainty = np.nan * unit.kilocalories_per_mole | ||
| else: | ||
| complex_dg_uncertainty = ( | ||
| np.std(complex_data) * unit.kilocalories_per_mole |
There was a problem hiding this comment.
| np.std(complex_data) * unit.kilocalories_per_mole | |
| np.std(complex_data, ddof=1) * unit.kilocalories_per_mole |
| solvent_dg_uncertainty = np.nan * unit.kilocalories_per_mole | ||
| else: | ||
| solvent_dg_uncertainty = ( | ||
| np.std(solvent_data) * unit.kilocalories_per_mole |
There was a problem hiding this comment.
| np.std(solvent_data) * unit.kilocalories_per_mole | |
| np.std(solvent_data, ddof=1) * unit.kilocalories_per_mole |
| if complex_data and solvent_data: | ||
| complex_dg = np.mean(complex_data) * unit.kilocalories_per_mole | ||
| if len(complex_data) == 1: | ||
| complex_dg_uncertainty = np.nan * unit.kilocalories_per_mole |
There was a problem hiding this comment.
This will stop cinnabar from running, if we have no repeat uncertainty, we should report the mbar error, we should probably report this for each repeat as well this was missed from the first submission :D
There was a problem hiding this comment.
This won't stop cinnabar anymore, because it will use the mbar uncertainty instead.
| gathered_results["dg"].append(entry_data) | ||
| except Exception as e: | ||
| logger.warning( | ||
| f"Could not generate absolute values (DG) for the alchemical map: {e}. " |
There was a problem hiding this comment.
We can still report DGs using the mbar error as the edge uncertainty, but we need to make this clear in the results.
There was a problem hiding this comment.
@jthorton in the dogs it lists as NaN. Maybe we add something to the dg result with a field like uncertainty_used='mbar'/'replicate'?
I am not sure about this one @IAlibay might know if this is possible.
Yes I think the reported uncertainty which we currently have as the std of the repeats, can be NaN or 0.0, but we should also report the estimator error for each repeat in the |
| alchemical_archives = [_load_archive(arch) for arch in archive] | ||
| network_obj = alchemical_archives[0].network | ||
|
|
||
| # Verify all archives have the same network |
There was a problem hiding this comment.
Why do all archives have to use the same network I think it would be fine to let them be different and just combine the results for example if we have run all of the jacs systems and have an archive per system we should be able to pass them all to this script.
| # Write to temporary file for loading | ||
| with tempfile.NamedTemporaryFile(mode="w", suffix=".json", delete=False) as tmp: | ||
| tmp.write(json_content) | ||
| tmp_path = tmp.name | ||
| else: | ||
| tmp_path = None |
There was a problem hiding this comment.
I think you can skip this step and pass the raw json to the from_json method.
There was a problem hiding this comment.
Sorry I didn't notice this one after fixing it in the generate script. Fixed now
| openfe_version: TODO | ||
| openmm_version: TODO | ||
| openff_toolkit_version: TODO |
| doi: TODO add DOI | ||
| archive_provider: TODO add archive provider |
| @@ -0,0 +1,83 @@ | |||
| #!/usr/bin/env python | |||
There was a problem hiding this comment.
Nice this is a great example of the API working!
There was a problem hiding this comment.
Do we want to save this to scripts as an example rather than in the results dir for this submission to try and keep the folders uniform?
For jacs_set/tyk2 with 1 repeat:
openfe-benchmarks/scripts/_tmp_alchemiscale_gather.pyto pull AlchemicalArchive from alchemiscaleopenfe-benchmarks/scripts/example_generate_results.pyto generatecomputational_results.jsonfrom the alchemical archiveopenfe-benchmarks/scripts/prepare_metadata_submission.pyto generate the zenodo description and submission.yamlQuestions: