Repository navigation
Conversation
Signed-off-by: Ohad Mosafi <omosafi@nvidia.com>
|
/nvskills-ci |
 [Medium impact] The changes since the previous review appear safe to merge; the four earlier findings remain non-blocking. Findings
SummaryThis PR prepares the protein binder workflow for signing and individual skill checks. It adds offline recovery helpers, evaluation fixtures, tests, and clearer campaign instructions. Since the previous review:
No new actionable issues were found in these changes. Diagram%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Load saved campaign] --> B{Required scores complete?}
B -->|Yes| C[Keep saved scores]
B -->|No| D[Check cached response and recorded hashes]
D -->|Match| E[Recover missing scores]
D -->|Missing or mismatch| F[Keep candidate blocked]
C --> G[Apply filters and save]
E --> G
F --> G
G --> H[Export full audit and ranked designs]
Reviews (9) · Last reviewed commit: "report judge refusals and diagnose inval..." · 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 |
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 |
Signed-off-by: Ohad Mosafi <omosafi@nvidia.com>
|
/nvskills-ci |
No description provided.