-
Notifications
You must be signed in to change notification settings - Fork 8
feat, fix(metada carrying): fix metadata dropping through the pipeline (atomworks, predictors), eliminate the post-fact patchign script. #301
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
3a13883
160dc0b
6448948
6330bd9
f05371b
53e3be4
f3681eb
6ba8aab
b615aa4
79e7b16
f5a0b9e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,11 @@ running, raising an error if it is not available. | |
|
|
||
| # Running the evaluations | ||
| ## Preparing the output CIF files | ||
|
|
||
| > Obsolete. Sampleworks now writes eval-ready `refined.cif` directly — the generation pipeline | ||
| > carries the depositor's real entity categories and the input residue numbering onto the output, so | ||
| > the metric scripts run against `refined.cif` with no preparation. The `patch_output_cif_files.py` | ||
| > step described below has been retired and removed; run the scripts against `refined.cif`. | ||
| As of this writing, Sampleworks outputs CIF files that primarily contain the output atomic | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably should remove the bit about requiring patching, since Sampleworks now outputs CiF files that should be complete. |
||
| coordinates, and not the additional information that many programs, like `tortoize` and | ||
| `phenix.clashscore`, require. Furthermore, many protein structure predictors effectively | ||
|
|
@@ -76,18 +81,16 @@ run_analysis altloc_classify | |
| run_analysis analyze_grid_search --jobs rscc --set defaults.PATCH_INPUT_PDB_PATTERN='{pdb_id}/{pdb_id}_original.cif' | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win Stale This example overrides a preset variable that this cohort's config changes remove ( 🤖 Prompt for AI Agents |
||
| ``` | ||
|
|
||
| The `analyze_grid_search`, `all`, and `external_tools` presets run the CIF patching pre-step before these | ||
| evaluation scripts. If you run the scripts directly, run `scripts/patch_output_cif_files.py` first | ||
| or point `--target-filename` at files that already contain the required metadata. If patched CIFs | ||
| already exist, add `--skip-pre-jobs` to rerun the analyses without repeating the patching step. | ||
| The `analyze_grid_search`, `all`, and `external_tools` presets run these evaluation scripts directly | ||
| on `refined.cif`; no patching pre-step is required. | ||
|
|
||
| For direct script invocation, the equivalent command shape is: | ||
|
|
||
| ```shell | ||
| pixi run -e analysis python scripts/eval/<script> \ | ||
| --grid-search-results-path /home/ubuntu/grid_search_results \ | ||
| --grid-search-inputs-path /home/ubuntu/grid_search_inputs \ | ||
| --target-filename 'refined-patched.cif' \ | ||
| --target-filename 'refined.cif' \ | ||
| --depth 4 \ | ||
| --protein-configs-csv /home/ubuntu/protein_analysis_config.csv \ | ||
| --occupancies 0.0 0.25 0.5 0.75 1.0 \ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,17 @@ | ||
| import argparse | ||
| import json | ||
| import subprocess | ||
| import tempfile | ||
| from pathlib import Path | ||
| from typing import Any | ||
|
|
||
| import joblib | ||
| import pandas as pd | ||
| from biotite.structure.io.pdbx.cif import CIFFile | ||
| from loguru import logger | ||
| from pandas import DataFrame | ||
| from sampleworks.eval.grid_search_eval_utils import parse_eval_args, setup_evaluation_parameters | ||
| from sampleworks.utils.cif_utils import extract_model, model_numbers | ||
|
|
||
|
|
||
| # TODO make more general: https://github.com/diff-use/sampleworks/issues/93 | ||
|
|
@@ -133,12 +136,17 @@ def get_protein_level_z_scores(tortoize_json: dict[str, Any]) -> pd.DataFrame: | |
|
|
||
| def get_stats_for_single_path(path: Path) -> tuple[DataFrame, DataFrame]: | ||
| """ | ||
| Run tortoize on a single CIF file and extract residue/protein-level stats. | ||
| Run tortoize per model on a single CIF file and extract residue/protein-level stats. | ||
|
|
||
| tortoize is single-model: it loads zero atoms from a stacked ensemble. Each model is extracted | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I recall differently. I am 99.9% sure tortoize outputs scores separately for each model. I know this because I have output that shows different values for each model, e.g., /mnt/diffuse-shared/raw/sampleworks/combined_occ_sweep_results/tortoize_residues.csv. You can also see that tortoize iterates over models here: https://github.com/PDB-REDO/tortoize/blob/trunk/src/tortoize.cpp#L1087 You should probably revert this particular change unless you can show that tortoize does not behave as advertised. |
||
| via a pure ``_atom_site`` filter (``extract_model``, which preserves the carried | ||
| ``_entity``/``_entity_poly``/``_entity_poly_seq`` categories), scored on its own, and the | ||
| per-model frames are merged -- each tagged with the source model number. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| path : Path | ||
| Path to a CIF file to process. | ||
| Path to a CIF file (single-model or stacked ensemble) to process. | ||
|
|
||
| Returns | ||
| ------- | ||
|
|
@@ -147,20 +155,45 @@ def get_stats_for_single_path(path: Path) -> tuple[DataFrame, DataFrame]: | |
| """ | ||
| logger.info(f"Processing {path}") | ||
| try: | ||
| output = subprocess.check_output(["tortoize", str(path)]) | ||
| result = json.loads(output) | ||
| except (subprocess.CalledProcessError, json.JSONDecodeError, OSError) as e: | ||
| logger.error(f"Failed to process {path}: {e}") | ||
| cif = CIFFile.read(str(path)) | ||
| except Exception as e: # noqa: BLE001 - a bad file must not abort the whole grid-search eval | ||
| logger.error(f"Failed to read {path}: {e}") | ||
| return pd.DataFrame(), pd.DataFrame() | ||
|
|
||
| residues = flatten_residues(result) | ||
| if residues.empty: | ||
| residue_frames: list[DataFrame] = [] | ||
| protein_frames: list[DataFrame] = [] | ||
| for model_num in model_numbers(cif): | ||
| single = CIFFile.read(str(path)) | ||
| extract_model(single, model_num) | ||
| with tempfile.NamedTemporaryFile(suffix=".cif", delete=False) as handle: | ||
| model_cif = Path(handle.name) | ||
| try: | ||
| single.write(str(model_cif)) | ||
| output = subprocess.check_output(["tortoize", str(model_cif)]) | ||
| result = json.loads(output) | ||
| except (subprocess.CalledProcessError, json.JSONDecodeError, OSError) as e: | ||
| logger.error(f"Failed to process model {model_num} of {path}: {e}") | ||
| continue | ||
| finally: | ||
| model_cif.unlink(missing_ok=True) | ||
|
|
||
| residues = flatten_residues(result) | ||
| protein = get_protein_level_z_scores(result) | ||
| # tortoize labels a lone model "1"; tag rows with the real source model number. | ||
| if not residues.empty: | ||
| residues["model"] = str(model_num) | ||
| residue_frames.append(residues) | ||
| if not protein.empty: | ||
| protein["model"] = str(model_num) | ||
| protein_frames.append(protein) | ||
|
|
||
| if not residue_frames: | ||
| logger.warning(f"No residues found in {path}") | ||
| return pd.DataFrame(), pd.DataFrame() | ||
|
|
||
| residues = pd.concat(residue_frames, ignore_index=True) | ||
| residues["path"] = path | ||
|
|
||
| protein_level_stats = get_protein_level_z_scores(result) | ||
| protein_level_stats = pd.concat(protein_frames, ignore_index=True) | ||
| protein_level_stats["path"] = path | ||
| return residues, protein_level_stats | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It may confuse users who aren't familiar with the package to say no CIF-patching is needed, since they won't understand it was previously required. You might say "... so the previously-required CIF-patching step is no longer required"