Repository navigation
Conversation
Signed-off-by: Ohad Mosafi <omosafi@nvidia.com>
|
/nvskills-ci |
 [Medium impact] The latest setup changes appear safe to merge; the five previous findings remain non-blocking follow-ups. Findings
SummaryThe PR expands and signs the protein binder workflow, adds offline exercises and recovery helpers, and moves its source under Since the last review:
No new actionable issues were found. Diagram%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Select installed Python 3.10–3.13] --> B{uv installed?}
B -->|Yes| C[Create environment with uv]
B -->|No| D[Create environment with Python venv]
C --> E[Install pinned wheels with uv]
D --> F[Install pinned wheels with pip]
E --> G[Check imports and versions]
F --> G
G --> H[Start offline evaluation]
C -->|Failure| I[Stop with phase-specific exit code]
D -->|Failure| I
E -->|Failure| I
F -->|Failure| I
G -->|Failure| I
Reviews (7) · Last reviewed commit: "Use installed uv for PBD evaluation envi..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| scores["binder_plddt"] = binder_plddt( | ||
| text, | ||
| format, | ||
| binding["binder_chain"], | ||
| len(candidate["sequence"]), | ||
| binding["plddt_scale"], | ||
| ) |
There was a problem hiding this comment.
extract_scores() always reads binder pLDDT, even when it is already saved or its filter is disabled. For a candidate missing only RMSD, unusable confidence values in the cached structure block recovery even though its coordinates can supply the missing score. Extract only the missing metrics, while keeping the chain and digest checks.
| candidate.setdefault("artifacts", {})["complex"] = str( | ||
| path.relative_to(output_dir) | ||
| ) |
There was a problem hiding this comment.
Recovery replaces an existing artifacts.complex reference whenever it fills a missing score. An interrupted run can already have a saved complex but lack RMSD. This changes its recorded path despite the contract to preserve existing artifacts, and the new grader rejects that change. Keep existing references and record the recovered structure separately when needed.
| verify_run( | ||
| path, | ||
| source, | ||
| profile["candidates"], | ||
| ranked, | ||
| lambda ok, msg: checks.append((ok, msg)), | ||
| ) | ||
| except (OSError, ValueError, KeyError, TypeError) as exc: | ||
| checks.append((False, f"{path}: {type(exc).__name__}: {exc}")) | ||
| attempts.append(checks) | ||
| valid = next((a for a in attempts if a and all(ok for ok, _ in a)), None) | ||
| chosen = ( | ||
| valid | ||
| if valid is not None | ||
| else ( | ||
| max(attempts, key=lambda a: sum(ok for ok, _ in a)) if attempts else [] | ||
| ) | ||
| ) | ||
| check(bool(chosen), f"{profile['name']}: output manifest found") | ||
| for ok, msg in chosen: | ||
| check(ok, msg) |
There was a problem hiding this comment.
The four bookkeeping cases never check reported counts or missing-score disclosures. verify_bookkeeping() checks the manifest and CSVs without reading summary.json or checking saved summary fields. Correct tables can earn artifact_correctness=1 alongside wrong counts or a falsely complete passing fraction. Add checks for these results and tests that corrupt them.
| and s.get("self_consistency_rmsd") is not None | ||
| ): | ||
| checks.append(s["self_consistency_rmsd"] <= f["self_consistency_rmsd_max"]) | ||
| required = self.required_metrics(c["id"]) |
There was a problem hiding this comment.
apply_filters() now searches the full candidate list for every candidate through required_metrics() and _find(). Doubling the campaign size roughly quadruples that search work. export_run() repeats the searches when collecting missing scores. Pass the candidate already in hand to an internal helper, or build an ID lookup once.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Signed-off-by: Ohad Mosafi <omosafi@nvidia.com>
|
/nvskills-ci |
Signed-off-by: Ohad Mosafi <omosafi@nvidia.com>
|
/nvskills-ci |
1 similar comment
|
/nvskills-ci |
Signed-off-by: Ohad Mosafi <omosafi@nvidia.com>
|
/nvskills-ci |
1 similar comment
|
/nvskills-ci |
Signed-off-by: Ohad Mosafi <omosafi@nvidia.com>
|
/nvskills-ci |
|
/nvskills-ci |
|
|
||
| ## Prerequisites | ||
|
|
||
| - Python 3.10-3.13; install [requirements.txt](requirements.txt) for RMSD, mmCIF, |
There was a problem hiding this comment.
The prerequisites now advertise Python 3.10–3.13, but compatibility still says Python>=3.12. Users on Python 3.10 or 3.11 get conflicting guidance about whether their environment is supported. Update compatibility in both the source and aggregate copies to match the supported range.
Signed-off-by: Ohad Mosafi <omosafi@nvidia.com>
|
/nvskills-ci |
1 participant
No description provided.