Julenmenideta/milab 6355 first version#1
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces the 'Sequence Embeddings' block, which generates per-sequence vector embeddings for peptide and antibody/TCR datasets using the ESM-2 protein language model. The implementation spans a Python runner for model inference, a Tengo workflow for batch orchestration and stats reporting, and a Vue-based UI for configuration and reporting. The review feedback highlights three key issues: a critical runtime AttributeError in the Python script due to a non-existent empty_table() method on pyarrow.Schema, a caching issue in the Tengo workflow caused by non-deterministic map iteration, and a potential crash in restricted container environments when calling os.sched_getaffinity(0) without catching general exceptions. Addressing these issues will ensure runtime stability and deterministic caching.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| def detect_cpu_budget() -> int: | ||
| """Best-effort allocated-CPU count. Prefers the cgroup-aware affinity set | ||
| (respects cpuset pinning), falling back to os.cpu_count(). NOTE: this does | ||
| NOT see CFS quota limits, so inside a Platforma exec the workflow should pass | ||
| the real allocation via --cpus rather than rely on this.""" | ||
| try: | ||
| return max(1, len(os.sched_getaffinity(0))) | ||
| except AttributeError: # os.sched_getaffinity is Linux-only | ||
| return os.cpu_count() or 1 |
There was a problem hiding this comment.
In restricted container environments or certain security configurations, os.sched_getaffinity(0) can raise a PermissionError or OSError even if the attribute exists. Catching Exception instead of only AttributeError ensures the fallback to os.cpu_count() is always safely reached.
| def detect_cpu_budget() -> int: | |
| """Best-effort allocated-CPU count. Prefers the cgroup-aware affinity set | |
| (respects cpuset pinning), falling back to os.cpu_count(). NOTE: this does | |
| NOT see CFS quota limits, so inside a Platforma exec the workflow should pass | |
| the real allocation via --cpus rather than rely on this.""" | |
| try: | |
| return max(1, len(os.sched_getaffinity(0))) | |
| except AttributeError: # os.sched_getaffinity is Linux-only | |
| return os.cpu_count() or 1 | |
| def detect_cpu_budget() -> int: | |
| """Best-effort allocated-CPU count. Prefers the cgroup-aware affinity set | |
| (respects cpuset pinning), falling back to os.cpu_count(). NOTE: this does | |
| NOT see CFS quota limits, so inside a Platforma exec the workflow should pass | |
| the real allocation via --cpus rather than rely on this.""" | |
| try: | |
| return max(1, len(os.sched_getaffinity(0))) | |
| except Exception: # Handle AttributeError (non-Linux) or OSError (restricted environments) | |
| return os.cpu_count() or 1 |
| log_message("device 'gpu' requested but CUDA is unavailable at runtime; " | ||
| "running the mounted checkpoint on CPU (fp32). Throughput will be lower.", | ||
| "WARNING") | ||
| # Apple Silicon's Metal backend | ||
| mps = getattr(torch.backends, "mps", None) | ||
| if mps is not None and mps.is_available(): | ||
| return torch.device("mps") | ||
|
|
||
| return torch.device("cpu") |
There was a problem hiding this comment.
Misleading GPU-fallback log message when MPS is available
The warning is emitted before the MPS check, so on an Apple Silicon machine where requested="gpu" but CUDA is absent and MPS is available, the log says "running the mounted checkpoint on CPU (fp32)" — yet the function returns torch.device("mps") on the next branch. An operator reading the log would expect CPU inference but the run actually uses the MPS accelerator. The warning should be deferred until after MPS has been excluded.
Prompt To Fix With AI
This is a comment left during a code review.
Path: software/src_python/main.py
Line: 147-155
Comment:
**Misleading GPU-fallback log message when MPS is available**
The warning is emitted before the MPS check, so on an Apple Silicon machine where `requested="gpu"` but CUDA is absent and MPS is available, the log says "running the mounted checkpoint on CPU (fp32)" — yet the function returns `torch.device("mps")` on the next branch. An operator reading the log would expect CPU inference but the run actually uses the MPS accelerator. The warning should be deferred until after MPS has been excluded.
How can I resolve this? If you propose a fix, please make it concise.| except Exception as exc: # noqa: BLE001 — log for the processing log, then re-raise | ||
| # A real embedding failure (model / CUDA / unrecoverable OOM / | ||
| # unexpected error) must surface as a BLOCK error, never silent or | ||
| # partial output. Log it, then re-raise: the non-zero exit fails | ||
| # the exec and the partial parquets are discarded with it. | ||
| log_message(f"embedding failed on chunk {ci}: {exc!r}", "ERROR") |
There was a problem hiding this comment.
Chunk index in the error log is 0-based (
ci) while all other chunk references in this function use the 1-based form (ci + 1). An operator matching the error log to the step log would look for the wrong chunk number.
| except Exception as exc: # noqa: BLE001 — log for the processing log, then re-raise | |
| # A real embedding failure (model / CUDA / unrecoverable OOM / | |
| # unexpected error) must surface as a BLOCK error, never silent or | |
| # partial output. Log it, then re-raise: the non-zero exit fails | |
| # the exec and the partial parquets are discarded with it. | |
| log_message(f"embedding failed on chunk {ci}: {exc!r}", "ERROR") | |
| except Exception as exc: # noqa: BLE001 — log for the processing log, then re-raise | |
| # A real embedding failure (model / CUDA / unrecoverable OOM / | |
| # unexpected error) must surface as a BLOCK error, never silent or | |
| # partial output. Log it, then re-raise: the non-zero exit fails | |
| # the exec and the partial parquets are discarded with it. | |
| log_message(f"embedding failed on chunk {ci + 1}: {exc!r}", "ERROR") |
Prompt To Fix With AI
This is a comment left during a code review.
Path: software/src_python/main.py
Line: 669-674
Comment:
Chunk index in the error log is 0-based (`ci`) while all other chunk references in this function use the 1-based form (`ci + 1`). An operator matching the error log to the step log would look for the wrong chunk number.
```suggestion
except Exception as exc: # noqa: BLE001 — log for the processing log, then re-raise
# A real embedding failure (model / CUDA / unrecoverable OOM /
# unexpected error) must surface as a BLOCK error, never silent or
# partial output. Log it, then re-raise: the non-zero exit fails
# the exec and the partial parquets are discarded with it.
log_message(f"embedding failed on chunk {ci + 1}: {exc!r}", "ERROR")
```
How can I resolve this? If you propose a fix, please make it concise.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!
| /* | ||
| There are no tests yet, create them via blockTest from @platforma-open/sdk-test' function. | ||
| */ |
There was a problem hiding this comment.
Test file has no meaningful assertions
The file only imports the workflow module for a syntax check. There are no describe/it blocks or assertions, so the test suite gives no coverage of the model logic, scope detection, or block args validation.
Prompt To Fix With AI
This is a comment left during a code review.
Path: test/src/wf.test.ts
Line: 1-3
Comment:
**Test file has no meaningful assertions**
The file only imports the workflow module for a syntax check. There are no `describe`/`it` blocks or assertions, so the test suite gives no coverage of the model logic, scope detection, or block args validation.
How can I resolve this? If you propose a fix, please make it concise.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!
Greptile Summary
This is the initial version of the Sequence Embeddings block — a Platforma workflow that embeds amino-acid sequences (CDR3, VDJRegion, Fv, scFv, peptide) using ESM-2 language models (150M / 650M) via HuggingFace Transformers, streaming the keyspace in chunks to bound peak memory, and writing long-format Parquet per scope for downstream PColumn import.
main.py): loads a single ESM-2 checkpoint, mean-pools penultimate-layer hidden states, supports CPU fan-out viamultiprocessing.Pool, and a stats-only counting pass (--stats-only) for the run-report step.processColumnper selected scope in batch mode; a separatecompute-statstemplate handles the run-report count; device/model selection (GPU/CPU, 650M/150M) is resolved at plan time.BlockModelV3wires input anchor detection, scope picker config (availableScopes), GPU-availability prerun, and stale-results detection;scopes.tsderives picker options from discovered sequence columns.Confidence Score: 4/5
The block is largely well-structured; one issue in the Python runner produces a false warning log that could mislead operators on Apple Silicon machines but does not affect inference correctness.
The device-fallback warning in resolve_device fires before the MPS check, so on Apple Silicon with no CUDA the log claims 'running on CPU' while the run actually uses the MPS accelerator. The wrong device string propagates into operator logs and could mislead any triage or performance investigation. All other code paths — chunk streaming, batch processColumn, scope detection, stats-only pass, and the UI — appear correct.
software/src_python/main.py — specifically the resolve_device fallback warning ordering around lines 147–155.
Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant UI as UI (Vue) participant Model as Model Layer (TS) participant Main as main.tpl.tengo participant EmbedTpl as compute-embeddings.tpl.tengo participant StatsTpl as compute-stats.tpl.tengo participant Py as main.py (Python) UI->>Model: inputAnchor + selectedScopes + fidelity Model->>Model: args() validate, sort scopes Model->>Main: BlockArgs Main->>Main: resolve device + modelTag loop per selected scope Main->>EmbedTpl: processColumn (batch mode) EmbedTpl->>Py: batch.tsv + plan.json (--input/--plan) Py->>Py: embed_unique() mean-pool penultimate layer Py-->>EmbedTpl: "embeddings_{scope}.parquet" EmbedTpl-->>Main: embedding PColumn chunk end Main->>StatsTpl: render.create (cols, scopes, keyAxisSpec) StatsTpl->>Py: source.tsv + plan.json (--stats-only) Py-->>StatsTpl: stats.json StatsTpl-->>Main: stats output Main-->>Model: outputs.stats + outputs.embeddings Model-->>UI: stats (WorkflowStats), isRunning, resultsStale%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant UI as UI (Vue) participant Model as Model Layer (TS) participant Main as main.tpl.tengo participant EmbedTpl as compute-embeddings.tpl.tengo participant StatsTpl as compute-stats.tpl.tengo participant Py as main.py (Python) UI->>Model: inputAnchor + selectedScopes + fidelity Model->>Model: args() validate, sort scopes Model->>Main: BlockArgs Main->>Main: resolve device + modelTag loop per selected scope Main->>EmbedTpl: processColumn (batch mode) EmbedTpl->>Py: batch.tsv + plan.json (--input/--plan) Py->>Py: embed_unique() mean-pool penultimate layer Py-->>EmbedTpl: "embeddings_{scope}.parquet" EmbedTpl-->>Main: embedding PColumn chunk end Main->>StatsTpl: render.create (cols, scopes, keyAxisSpec) StatsTpl->>Py: source.tsv + plan.json (--stats-only) Py-->>StatsTpl: stats.json StatsTpl-->>Main: stats output Main-->>Model: outputs.stats + outputs.embeddings Model-->>UI: stats (WorkflowStats), isRunning, resultsStalePrompt To Fix All With AI
Reviews (1): Last reviewed commit: "Changeset" | Re-trigger Greptile