Add self-consistency inferlet + GoT batching + GSM8K benchmark harness#461
Add self-consistency inferlet + GoT batching + GSM8K benchmark harness#461zatchbell1311-wq wants to merge 8 commits into
Conversation
…llision crash Pie's portable driver allocates a fresh ggml context arena per request. ggml's CUDA graph cache keys captured graphs by the raw pointer address of the graph's first node (cgraph->nodes[0]) — when a new request's first-node address happens to coincide with a previous (now-freed) request's address, the cache incorrectly treats the new, differently- shaped graph as identical to the cached one, skips rebuilding it, and replays a stale CUDA graph against mismatched tensor shapes. This crashed pie-server reliably (via different GGML_ASSERT failures depending on which shapes collided) after 1-2 successful requests to the same inferlet, or under moderate concurrency. This is a known upstream ggml/llama.cpp issue (ggml-org/llama.cpp #20315, #19708) — the maintainers' sanctioned workaround is disabling CUDA graph capture via GGML_CUDA_GRAPHS=OFF, which this commit forces at the Pie build.rs level for the portable driver's CUDA backend. Verified: before this fix, 6 identical repeated requests to the same inferlet crashed the server on request 2 every time. After, 6 repeated requests (self-consistency, 4-8 concurrent proposal contexts) and 3 repeated requests (graph-of-thought, 14 generation calls each) all completed cleanly with consistent correct outputs and no crashes. Tradeoff: disables a ~10-15% raw decode throughput optimization in exchange for correctness. Given crashes make the backend unusable for any sustained workload, this is a clear net win; a more surgical fix (incorporating cgraph->n_nodes or another shape-derived value into the cache key, matching the graph_recompute fix suggested upstream) could restore graph-reuse performance for genuinely-repeated shapes as a follow-up.
…mark harness - New self-consistency inferlet (8 parallel proposals, majority vote) - Batch GoT/SC proposal generation in chunks of 4 to reduce peak VRAM - Add benchmark_gsm8k.py: crash-resilient harness comparing SC vs GoT at matched token budgets (~3584 tokens), with incremental result saving - Add gsm8k_test.jsonl, analyze_results.py, install_inferlets.py - Results: SC 47.4% vs GoT 23.7% accuracy on 38 valid questions (12/50 excluded due to WSL2/CUDA infra crashes, unrelated to model behavior)
WalkthroughThis PR adds GSM8K benchmarking and analysis scripts, recorded benchmark datasets, a self-consistency inferlet, a reworked graph-of-thought inferlet, and CUDA/portable build configuration changes. ChangesGSM8K Benchmark and Analysis Tooling
New Self-Consistency Inferlet
Graph-of-Thought Aggregation Rework
Build Configuration Changes
Related Issues: None referenced in the provided information. Sequence Diagram(s)sequenceDiagram
participant benchmark_gsm8k.py
participant pie serve
participant self-consistency inferlet
participant graph-of-thought inferlet
benchmark_gsm8k.py->>pie serve: start / ping / restart
benchmark_gsm8k.py->>self-consistency inferlet: run baseline and sc questions
self-consistency inferlet-->>benchmark_gsm8k.py: predictions and tokens
benchmark_gsm8k.py->>graph-of-thought inferlet: run got question
graph-of-thought inferlet-->>benchmark_gsm8k.py: prediction and tokens
benchmark_gsm8k.py->>benchmark_gsm8k.py: save results and print metrics
sequenceDiagram
participant Input
participant main
participant Model
participant majority_vote
Input->>main: question and proposal_tokens
main->>Model: generate proposal texts
main->>main: strip think blocks and extract answers
main->>majority_vote: parsed numeric answers
majority_vote-->>main: winner or none
Poem: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
♻️ Duplicate comments (1)
benchmark_results.json (1)
1-401: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winRecorded data can't distinguish infrastructure failures from genuine NO_CONSENSUS.
Rows where a question failed due to server crashes (per the PR description, 12/50 questions) are stored with
sc_pred/got_predasnull, identical to rows where the model itself failed to reach consensus. This is a direct downstream consequence ofparse_predictioninbenchmark_gsm8k.pynot distinguishing the"SERVER_ERROR"sentinel from a realNO_CONSENSUSresult — see the corresponding comment on that file. Without a status field, this dataset can't be reliably re-analyzed to separate the two causes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benchmark_results.json` around lines 1 - 401, The benchmark output currently mixes server crashes with genuine NO_CONSENSUS cases because `parse_prediction` in `benchmark_gsm8k.py` converts the `"SERVER_ERROR"` sentinel into the same `null` prediction shape as a real consensus failure. Update the data recording path to preserve the outcome type for each row, using a distinct status/flag from `parse_prediction` and the benchmark result writer so `sc_pred`/`got_pred` no longer lose error provenance. Make sure the JSON emitted for each question clearly distinguishes infrastructure failure from model disagreement without relying on `null` alone.
🧹 Nitpick comments (5)
inferlets/graph-of-thought/src/lib.rs (1)
114-121: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDead constant
AGGREGATE_PROMPT.The constant is empty and self-described as unused; the prompt is built inline in
launch_aggregation_pairs. Consider removing it to avoid confusion.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@inferlets/graph-of-thought/src/lib.rs` around lines 114 - 121, The empty unused constant AGGREGATE_PROMPT in lib.rs should be removed to avoid dead code and confusion. Update the prompt handling in the graph-of-thought module by deleting AGGREGATE_PROMPT and relying on the existing format_aggregate_prompt path used by launch_aggregation_pairs; keep SYSTEM_PROMPT and PROPOSAL_PROMPT_TEMPLATE unchanged.install_inferlets.py (1)
1-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicates hardcoded install logic already present in
benchmark_gsm8k.py'sinstall_inferlets().Both this file and
install_inferlets()(benchmark_gsm8k.py, lines 105-109) hardcode the same/home/dhruv/...wasm/manifest paths and perform the identical two-step install. Consider havingbenchmark_gsm8k.pyimport/reuse this script's logic (or vice versa) to avoid the two copies drifting out of sync, and address the portability concern (hardcoded absolute paths) in one place.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@install_inferlets.py` around lines 1 - 20, The install flow is duplicated with hardcoded absolute paths, so update the shared install logic instead of keeping two copies in sync. Refactor the existing main() in install_inferlets.py or benchmark_gsm8k.py’s install_inferlets() to reuse a single helper for the two client.install_program calls, and centralize the wasm/manifest paths so they are defined once and can be made portable.server/build.rs (1)
133-141: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winComment now contradicts the code — update to explain why CUDA graphs are disabled.
Lines 135-139 justify enabling CUDA graphs for perf, but line 140 now sets
GGML_CUDA_GRAPHSto"OFF". Per the PR's commit context, this was an intentional fix for a CUDA-graph pointer-key-collision crash under Pie's request-per-context model — but the comment still describes the opposite rationale, which will confuse future maintainers into re-enabling this and reintroducing the crash.📝 Suggested comment update
- // ggml's CUDA-graph capture/replay is OFF by default at the ggml- - // subproject level; llama.cpp's parent CMakeLists.txt flips it - // ON. Embedding ggml directly (as we do here) misses that override - // and ends up issuing ~30 kernel launches per decode step, which - // costs ~1 ms vs ~0.3 ms with capture. Enable explicitly. - .define("GGML_CUDA_GRAPHS", "OFF") + // CUDA-graph capture/replay is disabled: under Pie's + // request-per-context model, repeated graph capture/replay + // triggers a pointer-key collision in ggml's CUDA graph node + // cache, causing crashes on repeated requests. Costs ~1ms vs + // ~0.3ms per decode step without capture; re-enable only after + // the upstream key-collision issue is fixed. + .define("GGML_CUDA_GRAPHS", "OFF")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/build.rs` around lines 133 - 141, The comment in the CUDA config block contradicts the actual GGML_CUDA_GRAPHS setting in build.rs. Update the explanatory comment around the cfg.define chain in the CUDA-enabled branch to describe why CUDA graphs are intentionally set to OFF, referencing the Pie request-per-context pointer-key-collision crash rationale instead of the performance claim, so future changes to cuda_enabled and GGML_CUDA_GRAPHS are not misled.inferlets/self-consistency/Pie.toml (1)
13-13: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDoc says "must be even" but this isn't enforced anywhere.
The description states the
proposal_tokensarray length "must be even," butinferlets/self-consistency/src/lib.rsnever validates this — an odd-length array (e.g. 3 proposals) is silently accepted and works fine for majority voting. Either enforce the constraint or drop the misleading wording.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@inferlets/self-consistency/Pie.toml` at line 13, The Pie.toml description for proposal_tokens says the proposal count “must be even,” but SelfConsistency in inferlets/self-consistency/src/lib.rs does not enforce that rule and odd-length proposal arrays are accepted. Update the documentation to remove the misleading “must be even” wording, or add a validation check in the SelfConsistency parsing/handling path to reject odd proposal counts so the config and runtime behavior match.inferlets/self-consistency/src/lib.rs (1)
90-105: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTwo divergent vote-grouping algorithms.
majority_vote(Lines 90-105) groups floats using an epsilon comparison (abs() < 1e-9), whilevote_counts(Lines 162-165) groups the same values using aformat!("{:.6}", ans)string key. These two groupings can diverge for edge-case floats (e.g., values differing by less than 1e-9 but rounding to different 6-decimal strings, or vice versa), making the printed "Vote counts" diagnostic potentially inconsistent with the actual winner. Consider derivingvote_countsfrom the same grouping used bymajority_voteto keep the debug output and decision logic in sync.Also applies to: 162-166
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@inferlets/self-consistency/src/lib.rs` around lines 90 - 105, `majority_vote` and the `vote_counts` diagnostic are using different float-grouping rules, which can make the displayed counts disagree with the selected winner. Update `vote_counts` in `infer_one` to derive its groups from the same epsilon-based comparison logic used by `majority_vote` (or factor that grouping into a shared helper) so both the debug output and the vote decision stay consistent. Use the existing `majority_vote` and `vote_counts` symbols to keep the grouping behavior aligned for all `Option<f64>` answers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@analyze_results.py`:
- Around line 8-29: Guard the summary printing in analyze_results.py against an
empty rows list before computing percentages. In the main analysis block where
n, sc_correct, got_correct, sc_no_consensus, and got_no_consensus are used, add
an early exit or conditional branch for n == 0 so the f-strings in the
Self-consistency and Graph-of-Thought sections never divide by zero. Keep the
existing output structure in the non-empty case and reference the same
counters/metrics already computed.
In `@benchmark_gsm8k.py`:
- Around line 190-198: The aggregate summary in the results block uses
n_questions as the denominator even though the actual evaluated data is in rows,
which can become stale when resuming with fewer questions than an existing run.
Update the summary logic near the results printout in benchmark_gsm8k.py to
compute totals and percentages from len(rows) (or otherwise clamp to the
evaluated row count) instead of n_questions, and keep the displayed N consistent
with the actual number of processed rows. Use the existing variables sc_correct,
got_correct, sc_no_consensus, got_no_consensus, and rows to locate the fix.
- Around line 69-79: The start_server function leaks the SERVE_LOG file handle
because log_f is opened in the parent and never closed after subprocess.Popen
starts the server. Update start_server so the parent closes or otherwise manages
the log file handle immediately after spawning the child, while still preserving
stdout/stderr redirection via PIE_BIN and Popen. Make the fix in start_server
and ensure restart_server_and_wait can call it repeatedly without accumulating
open descriptors.
- Around line 13-21: Replace the developer-specific absolute constants in
benchmark_gsm8k.py with paths derived from the repository/runtime environment
instead of /home/dhruv/... so the script works across machines and CI. Update
the path setup near GSM8K_PATH, RESULTS_PATH, CONFIG_PATH, SERVE_LOG, PIE_BIN,
SC_WASM, SC_MANIFEST, GOT_WASM, and GOT_MANIFEST to be computed from the project
root or configurable via environment variables/CLI arguments. Keep the existing
symbol names if helpful, but make their values portable and
checkout-independent.
- Around line 48-54: The current parse_prediction flow collapses SERVER_ERROR
and NO_CONSENSUS into the same None result, so update parse_prediction and the
downstream result handling in benchmark_gsm8k/analyze_results to preserve a
separate error/status marker for SERVER_ERROR. Keep NO_CONSENSUS mapped as the
model non-answer, but store SERVER_ERROR distinctly in the saved results and
budget-artifact counts so analyze_results can count it separately instead of
treating all None values the same.
In `@driver/cuda/CMakeLists.txt`:
- Around line 370-438: The fallback branch in the PIE_CUDA_BUILD_MOE block
references a non-existent stub source, so the default OFF path breaks the build.
Update the else branch around target_sources(pie_driver_cuda_lib) to point at
the actual stub implementation used for flashinfer_moe, or add the missing stub
file under the existing src/ops naming expected by flashinfer_moe_stub.cpp. Make
sure the symbols PIE_CUDA_BUILD_MOE, pie_driver_cuda_lib, and
flashinfer_moe_stub.cpp all resolve consistently.
In `@driver/portable/src/graph_common.hpp`:
- Around line 22-24: GRAPH_MAX_NODES is now too low for supported Qwen3
large-batch paths and can under-size both graph construction and the backend
scheduler buffer. Update the constant in graph_common.hpp to preserve a much
larger headroom, or refactor the budget to be model/batch-specific using the
existing graph_qwen3.cpp sizing logic so large request counts do not fail at
runtime.
In `@inferlets/graph-of-thought/src/lib.rs`:
- Around line 94-112: The tie-break behavior in majority_vote is documented as
first-occurrence, but max_by_key will pick the last maximal entry among ties.
Update majority_vote so the tie-break is explicit and matches the intended
first-seen order, or adjust the doc comment to reflect the actual
last-occurrence behavior; keep the change localized to majority_vote and its
comment.
- Around line 74-92: `extract_final_answer` is slicing from `cleaned` using an
index found in the lowercased buffer, which can misalign for Unicode because
`to_lowercase()` may change byte length. Update `extract_final_answer` to derive
the substring from the same lowercase string used for `rfind`, or otherwise map
the index safely before slicing, and make the same correction in the matching
`extract_final_answer` implementation in
`inferlets/self-consistency/src/lib.rs`.
In `@inferlets/self-consistency/Pie.toml`:
- Line 13: The manifest schema for proposal_tokens is declared as a scalar
string, but inferlets/self-consistency/src/lib.rs expects a Vec<usize>, so
update the Pie.toml declaration to a list-capable array schema for
proposal_tokens or align the inferlet deserialization to a scalar format. Use
the proposal_tokens field definition in Pie.toml and the corresponding parsing
logic in the self-consistency lib as the matching points, and make sure the
manifest-driven input shape matches the actual JSON array expected by the
inferlet.
In `@inferlets/self-consistency/src/lib.rs`:
- Around line 67-85: `extract_final_answer` is slicing `cleaned` using an index
computed from `to_lowercase()`, which can become invalid for Unicode text;
update the marker search to use a byte-preserving approach or a case-insensitive
search on the original string, and then derive the `after` slice from the same
source string so `rfind`/slice offsets stay aligned.
---
Duplicate comments:
In `@benchmark_results.json`:
- Around line 1-401: The benchmark output currently mixes server crashes with
genuine NO_CONSENSUS cases because `parse_prediction` in `benchmark_gsm8k.py`
converts the `"SERVER_ERROR"` sentinel into the same `null` prediction shape as
a real consensus failure. Update the data recording path to preserve the outcome
type for each row, using a distinct status/flag from `parse_prediction` and the
benchmark result writer so `sc_pred`/`got_pred` no longer lose error provenance.
Make sure the JSON emitted for each question clearly distinguishes
infrastructure failure from model disagreement without relying on `null` alone.
---
Nitpick comments:
In `@inferlets/graph-of-thought/src/lib.rs`:
- Around line 114-121: The empty unused constant AGGREGATE_PROMPT in lib.rs
should be removed to avoid dead code and confusion. Update the prompt handling
in the graph-of-thought module by deleting AGGREGATE_PROMPT and relying on the
existing format_aggregate_prompt path used by launch_aggregation_pairs; keep
SYSTEM_PROMPT and PROPOSAL_PROMPT_TEMPLATE unchanged.
In `@inferlets/self-consistency/Pie.toml`:
- Line 13: The Pie.toml description for proposal_tokens says the proposal count
“must be even,” but SelfConsistency in inferlets/self-consistency/src/lib.rs
does not enforce that rule and odd-length proposal arrays are accepted. Update
the documentation to remove the misleading “must be even” wording, or add a
validation check in the SelfConsistency parsing/handling path to reject odd
proposal counts so the config and runtime behavior match.
In `@inferlets/self-consistency/src/lib.rs`:
- Around line 90-105: `majority_vote` and the `vote_counts` diagnostic are using
different float-grouping rules, which can make the displayed counts disagree
with the selected winner. Update `vote_counts` in `infer_one` to derive its
groups from the same epsilon-based comparison logic used by `majority_vote` (or
factor that grouping into a shared helper) so both the debug output and the vote
decision stay consistent. Use the existing `majority_vote` and `vote_counts`
symbols to keep the grouping behavior aligned for all `Option<f64>` answers.
In `@install_inferlets.py`:
- Around line 1-20: The install flow is duplicated with hardcoded absolute
paths, so update the shared install logic instead of keeping two copies in sync.
Refactor the existing main() in install_inferlets.py or benchmark_gsm8k.py’s
install_inferlets() to reuse a single helper for the two client.install_program
calls, and centralize the wasm/manifest paths so they are defined once and can
be made portable.
In `@server/build.rs`:
- Around line 133-141: The comment in the CUDA config block contradicts the
actual GGML_CUDA_GRAPHS setting in build.rs. Update the explanatory comment
around the cfg.define chain in the CUDA-enabled branch to describe why CUDA
graphs are intentionally set to OFF, referencing the Pie request-per-context
pointer-key-collision crash rationale instead of the performance claim, so
future changes to cuda_enabled and GGML_CUDA_GRAPHS are not misled.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 573db40e-a7fb-46cb-a218-2e8c7b089833
📒 Files selected for processing (12)
analyze_results.pybenchmark_gsm8k.pybenchmark_results.jsondriver/cuda/CMakeLists.txtdriver/portable/src/graph_common.hppgsm8k_test.jsonlinferlets/graph-of-thought/src/lib.rsinferlets/self-consistency/Cargo.tomlinferlets/self-consistency/Pie.tomlinferlets/self-consistency/src/lib.rsinstall_inferlets.pyserver/build.rs
| n = len(rows) | ||
| sc_correct = sum(r["sc_correct"] for r in rows) | ||
| got_correct = sum(r["got_correct"] for r in rows) | ||
| sc_no_consensus = sum(r["sc_pred"] is None for r in rows) | ||
| got_no_consensus = sum(r["got_pred"] is None for r in rows) | ||
|
|
||
| sc_answered = n - sc_no_consensus | ||
| got_answered = n - got_no_consensus | ||
|
|
||
| print(f"=== GSM8K Benchmark Analysis (N={n}) ===\n") | ||
|
|
||
| print("Self-consistency:") | ||
| print(f" Overall accuracy: {sc_correct}/{n} = {100*sc_correct/n:.1f}%") | ||
| print(f" NO_CONSENSUS rate: {sc_no_consensus}/{n} = {100*sc_no_consensus/n:.1f}%") | ||
| if sc_answered > 0: | ||
| print(f" Accuracy | answered: {sc_correct}/{sc_answered} = {100*sc_correct/sc_answered:.1f}%") | ||
|
|
||
| print("\nGraph-of-Thought:") | ||
| print(f" Overall accuracy: {got_correct}/{n} = {100*got_correct/n:.1f}%") | ||
| print(f" NO_CONSENSUS rate: {got_no_consensus}/{n} = {100*got_no_consensus/n:.1f}%") | ||
| if got_answered > 0: | ||
| print(f" Accuracy | answered: {got_correct}/{got_answered} = {100*got_correct/got_answered:.1f}%") |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Division by zero on empty results file.
If rows is empty (n == 0), lines computing 100*sc_correct/n etc. will raise ZeroDivisionError.
🔧 Suggested fix
n = len(rows)
+ if n == 0:
+ print("No results to analyze.")
+ return
sc_correct = sum(r["sc_correct"] for r in rows)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| n = len(rows) | |
| sc_correct = sum(r["sc_correct"] for r in rows) | |
| got_correct = sum(r["got_correct"] for r in rows) | |
| sc_no_consensus = sum(r["sc_pred"] is None for r in rows) | |
| got_no_consensus = sum(r["got_pred"] is None for r in rows) | |
| sc_answered = n - sc_no_consensus | |
| got_answered = n - got_no_consensus | |
| print(f"=== GSM8K Benchmark Analysis (N={n}) ===\n") | |
| print("Self-consistency:") | |
| print(f" Overall accuracy: {sc_correct}/{n} = {100*sc_correct/n:.1f}%") | |
| print(f" NO_CONSENSUS rate: {sc_no_consensus}/{n} = {100*sc_no_consensus/n:.1f}%") | |
| if sc_answered > 0: | |
| print(f" Accuracy | answered: {sc_correct}/{sc_answered} = {100*sc_correct/sc_answered:.1f}%") | |
| print("\nGraph-of-Thought:") | |
| print(f" Overall accuracy: {got_correct}/{n} = {100*got_correct/n:.1f}%") | |
| print(f" NO_CONSENSUS rate: {got_no_consensus}/{n} = {100*got_no_consensus/n:.1f}%") | |
| if got_answered > 0: | |
| print(f" Accuracy | answered: {got_correct}/{got_answered} = {100*got_correct/got_answered:.1f}%") | |
| n = len(rows) | |
| if n == 0: | |
| print("No results to analyze.") | |
| return | |
| sc_correct = sum(r["sc_correct"] for r in rows) | |
| got_correct = sum(r["got_correct"] for r in rows) | |
| sc_no_consensus = sum(r["sc_pred"] is None for r in rows) | |
| got_no_consensus = sum(r["got_pred"] is None for r in rows) | |
| sc_answered = n - sc_no_consensus | |
| got_answered = n - got_no_consensus | |
| print(f"=== GSM8K Benchmark Analysis (N={n}) ===\n") | |
| print("Self-consistency:") | |
| print(f" Overall accuracy: {sc_correct}/{n} = {100*sc_correct/n:.1f}%") | |
| print(f" NO_CONSENSUS rate: {sc_no_consensus}/{n} = {100*sc_no_consensus/n:.1f}%") | |
| if sc_answered > 0: | |
| print(f" Accuracy | answered: {sc_correct}/{sc_answered} = {100*sc_correct/sc_answered:.1f}%") | |
| print("\nGraph-of-Thought:") | |
| print(f" Overall accuracy: {got_correct}/{n} = {100*got_correct/n:.1f}%") | |
| print(f" NO_CONSENSUS rate: {got_no_consensus}/{n} = {100*got_no_consensus/n:.1f}%") | |
| if got_answered > 0: | |
| print(f" Accuracy | answered: {got_correct}/{got_answered} = {100*got_correct/got_answered:.1f}%") |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@analyze_results.py` around lines 8 - 29, Guard the summary printing in
analyze_results.py against an empty rows list before computing percentages. In
the main analysis block where n, sc_correct, got_correct, sc_no_consensus, and
got_no_consensus are used, add an early exit or conditional branch for n == 0 so
the f-strings in the Self-consistency and Graph-of-Thought sections never divide
by zero. Keep the existing output structure in the non-empty case and reference
the same counters/metrics already computed.
| GSM8K_PATH = "/home/dhruv/pie/gsm8k_test.jsonl" | ||
| RESULTS_PATH = "/home/dhruv/pie/benchmark_results.json" | ||
| CONFIG_PATH = "/home/dhruv/.pie/config.toml" | ||
| SERVE_LOG = "/home/dhruv/pie/serve.log" | ||
| PIE_BIN = "/home/dhruv/pie/target/release/pie" | ||
| SC_WASM = "/home/dhruv/pie/build-out/self_consistency.wasm" | ||
| SC_MANIFEST = "/home/dhruv/pie/inferlets/self-consistency/Pie.toml" | ||
| GOT_WASM = "/home/dhruv/pie/build-out/graph_of_thought.wasm" | ||
| GOT_MANIFEST = "/home/dhruv/pie/inferlets/graph-of-thought/Pie.toml" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Hardcoded developer-specific absolute paths.
All key paths (GSM8K_PATH, RESULTS_PATH, CONFIG_PATH, SERVE_LOG, PIE_BIN, wasm/manifest paths) are hardcoded to /home/dhruv/.... This makes the script unusable for anyone else without manual edits, and fragile if run from CI or another checkout location.
♻️ Suggested fix
-SERVER_URL = "ws://127.0.0.1:8080"
-GSM8K_PATH = "/home/dhruv/pie/gsm8k_test.jsonl"
-RESULTS_PATH = "/home/dhruv/pie/benchmark_results.json"
-CONFIG_PATH = "/home/dhruv/.pie/config.toml"
-SERVE_LOG = "/home/dhruv/pie/serve.log"
-PIE_BIN = "/home/dhruv/pie/target/release/pie"
-SC_WASM = "/home/dhruv/pie/build-out/self_consistency.wasm"
-SC_MANIFEST = "/home/dhruv/pie/inferlets/self-consistency/Pie.toml"
-GOT_WASM = "/home/dhruv/pie/build-out/graph_of_thought.wasm"
-GOT_MANIFEST = "/home/dhruv/pie/inferlets/graph-of-thought/Pie.toml"
+REPO_ROOT = Path(__file__).resolve().parent
+SERVER_URL = os.environ.get("PIE_SERVER_URL", "ws://127.0.0.1:8080")
+GSM8K_PATH = os.environ.get("GSM8K_PATH", str(REPO_ROOT / "gsm8k_test.jsonl"))
+RESULTS_PATH = os.environ.get("RESULTS_PATH", str(REPO_ROOT / "benchmark_results.json"))
+CONFIG_PATH = os.environ.get("PIE_CONFIG_PATH", str(Path.home() / ".pie" / "config.toml"))
+SERVE_LOG = os.environ.get("SERVE_LOG", str(REPO_ROOT / "serve.log"))
+PIE_BIN = os.environ.get("PIE_BIN", str(REPO_ROOT / "target" / "release" / "pie"))
+SC_WASM = str(REPO_ROOT / "build-out" / "self_consistency.wasm")
+SC_MANIFEST = str(REPO_ROOT / "inferlets" / "self-consistency" / "Pie.toml")
+GOT_WASM = str(REPO_ROOT / "build-out" / "graph_of_thought.wasm")
+GOT_MANIFEST = str(REPO_ROOT / "inferlets" / "graph-of-thought" / "Pie.toml")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| GSM8K_PATH = "/home/dhruv/pie/gsm8k_test.jsonl" | |
| RESULTS_PATH = "/home/dhruv/pie/benchmark_results.json" | |
| CONFIG_PATH = "/home/dhruv/.pie/config.toml" | |
| SERVE_LOG = "/home/dhruv/pie/serve.log" | |
| PIE_BIN = "/home/dhruv/pie/target/release/pie" | |
| SC_WASM = "/home/dhruv/pie/build-out/self_consistency.wasm" | |
| SC_MANIFEST = "/home/dhruv/pie/inferlets/self-consistency/Pie.toml" | |
| GOT_WASM = "/home/dhruv/pie/build-out/graph_of_thought.wasm" | |
| GOT_MANIFEST = "/home/dhruv/pie/inferlets/graph-of-thought/Pie.toml" | |
| REPO_ROOT = Path(__file__).resolve().parent | |
| SERVER_URL = os.environ.get("PIE_SERVER_URL", "ws://127.0.0.1:8080") | |
| GSM8K_PATH = os.environ.get("GSM8K_PATH", str(REPO_ROOT / "gsm8k_test.jsonl")) | |
| RESULTS_PATH = os.environ.get("RESULTS_PATH", str(REPO_ROOT / "benchmark_results.json")) | |
| CONFIG_PATH = os.environ.get("PIE_CONFIG_PATH", str(Path.home() / ".pie" / "config.toml")) | |
| SERVE_LOG = os.environ.get("SERVE_LOG", str(REPO_ROOT / "serve.log")) | |
| PIE_BIN = os.environ.get("PIE_BIN", str(REPO_ROOT / "target" / "release" / "pie")) | |
| SC_WASM = str(REPO_ROOT / "build-out" / "self_consistency.wasm") | |
| SC_MANIFEST = str(REPO_ROOT / "inferlets" / "self-consistency" / "Pie.toml") | |
| GOT_WASM = str(REPO_ROOT / "build-out" / "graph_of_thought.wasm") | |
| GOT_MANIFEST = str(REPO_ROOT / "inferlets" / "graph-of-thought" / "Pie.toml") |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@benchmark_gsm8k.py` around lines 13 - 21, Replace the developer-specific
absolute constants in benchmark_gsm8k.py with paths derived from the
repository/runtime environment instead of /home/dhruv/... so the script works
across machines and CI. Update the path setup near GSM8K_PATH, RESULTS_PATH,
CONFIG_PATH, SERVE_LOG, PIE_BIN, SC_WASM, SC_MANIFEST, GOT_WASM, and
GOT_MANIFEST to be computed from the project root or configurable via
environment variables/CLI arguments. Keep the existing symbol names if helpful,
but make their values portable and checkout-independent.
| def parse_prediction(raw: str): | ||
| if raw == "NO_CONSENSUS": | ||
| return None | ||
| try: | ||
| return float(raw) | ||
| except (ValueError, TypeError): | ||
| return None |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Files:\n'
git ls-files | rg '(^|/)(benchmark_gsm8k\.py|analyze_results\.py)$'
printf '\nbenchmark_gsm8k.py relevant lines:\n'
nl -ba benchmark_gsm8k.py | sed -n '1,260p'
printf '\nanalyze_results.py relevant lines:\n'
nl -ba analyze_results.py | sed -n '1,260p'Repository: pie-project/pie
Length of output: 274
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo 'benchmark_gsm8k.py:'
rg -n 'NO_CONSENSUS|SERVER_ERROR|parse_prediction|server_error|sc_raw|got_raw|analyze_results' benchmark_gsm8k.py
echo
echo 'analyze_results.py:'
rg -n 'NO_CONSENSUS|SERVER_ERROR|budget-artifact|server_error|parse_prediction|status|manual' analyze_results.pyRepository: pie-project/pie
Length of output: 1205
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
for path in [Path('benchmark_gsm8k.py'), Path('analyze_results.py')]:
print(f'### {path}')
text = path.read_text()
for needle in ['NO_CONSENSUS', 'SERVER_ERROR', 'parse_prediction', 'status', 'sc_raw', 'got_raw', 'budget-artifact', 'manual']:
if needle in text:
print(f'contains {needle}')
print()
PYRepository: pie-project/pie
Length of output: 326
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo 'benchmark_gsm8k.py lines 40-210:'
sed -n '40,210p' benchmark_gsm8k.py
echo
echo 'analyze_results.py lines 1-220:'
sed -n '1,220p' analyze_results.pyRepository: pie-project/pie
Length of output: 8223
Keep SERVER_ERROR distinct from NO_CONSENSUS. SERVER_ERROR is converted to None by parse_prediction(), and analyze_results.py counts every None as NO_CONSENSUS. That makes retry exhaustion indistinguishable from a real model non-answer in the saved results and budget-artifact counts; store a separate error/status field instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@benchmark_gsm8k.py` around lines 48 - 54, The current parse_prediction flow
collapses SERVER_ERROR and NO_CONSENSUS into the same None result, so update
parse_prediction and the downstream result handling in
benchmark_gsm8k/analyze_results to preserve a separate error/status marker for
SERVER_ERROR. Keep NO_CONSENSUS mapped as the model non-answer, but store
SERVER_ERROR distinctly in the saved results and budget-artifact counts so
analyze_results can count it separately instead of treating all None values the
same.
| def start_server(): | ||
| """Start pie serve in the background, wait for it to be ready.""" | ||
| log_f = open(SERVE_LOG, "a") | ||
| env = os.environ.copy() | ||
| env["GGML_CUDA_GRAPHS"] = "0" | ||
| proc = subprocess.Popen( | ||
| [PIE_BIN, "serve", "--config", CONFIG_PATH, "--no-auth"], | ||
| stdout=log_f, stderr=subprocess.STDOUT, | ||
| env=env, | ||
| ) | ||
| return proc |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
File handle leak on repeated start_server calls.
log_f is opened but never closed in the parent process; subprocess.Popen duplicates the fd for the child, but the parent's copy stays open indefinitely. Since start_server is called repeatedly by restart_server_and_wait in the crash-recovery loop, each restart leaks one file descriptor, which can accumulate over a long benchmark run with many crashes.
🔧 Suggested fix
def start_server():
"""Start pie serve in the background, wait for it to be ready."""
- log_f = open(SERVE_LOG, "a")
env = os.environ.copy()
env["GGML_CUDA_GRAPHS"] = "0"
- proc = subprocess.Popen(
- [PIE_BIN, "serve", "--config", CONFIG_PATH, "--no-auth"],
- stdout=log_f, stderr=subprocess.STDOUT,
- env=env,
- )
- return proc
+ with open(SERVE_LOG, "a") as log_f:
+ proc = subprocess.Popen(
+ [PIE_BIN, "serve", "--config", CONFIG_PATH, "--no-auth"],
+ stdout=log_f, stderr=subprocess.STDOUT,
+ env=env,
+ )
+ return proc📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def start_server(): | |
| """Start pie serve in the background, wait for it to be ready.""" | |
| log_f = open(SERVE_LOG, "a") | |
| env = os.environ.copy() | |
| env["GGML_CUDA_GRAPHS"] = "0" | |
| proc = subprocess.Popen( | |
| [PIE_BIN, "serve", "--config", CONFIG_PATH, "--no-auth"], | |
| stdout=log_f, stderr=subprocess.STDOUT, | |
| env=env, | |
| ) | |
| return proc | |
| def start_server(): | |
| """Start pie serve in the background, wait for it to be ready.""" | |
| env = os.environ.copy() | |
| env["GGML_CUDA_GRAPHS"] = "0" | |
| with open(SERVE_LOG, "a") as log_f: | |
| proc = subprocess.Popen( | |
| [PIE_BIN, "serve", "--config", CONFIG_PATH, "--no-auth"], | |
| stdout=log_f, stderr=subprocess.STDOUT, | |
| env=env, | |
| ) | |
| return proc |
🧰 Tools
🪛 ast-grep (0.44.1)
[error] 73-77: Command coming from incoming request
Context: subprocess.Popen(
[PIE_BIN, "serve", "--config", CONFIG_PATH, "--no-auth"],
stdout=log_f, stderr=subprocess.STDOUT,
env=env,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
[warning] 70-70: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(SERVE_LOG, "a")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
🪛 Ruff (0.15.20)
[error] 74-74: subprocess call: check for execution of untrusted input
(S603)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@benchmark_gsm8k.py` around lines 69 - 79, The start_server function leaks the
SERVE_LOG file handle because log_f is opened in the parent and never closed
after subprocess.Popen starts the server. Update start_server so the parent
closes or otherwise manages the log file handle immediately after spawning the
child, while still preserving stdout/stderr redirection via PIE_BIN and Popen.
Make the fix in start_server and ensure restart_server_and_wait can call it
repeatedly without accumulating open descriptors.
| sc_correct = sum(r["sc_correct"] for r in rows) | ||
| got_correct = sum(r["got_correct"] for r in rows) | ||
| sc_no_consensus = sum(r["sc_pred"] is None for r in rows) | ||
| got_no_consensus = sum(r["got_pred"] is None for r in rows) | ||
|
|
||
| print("\n=== Results ===") | ||
| print(f"N = {n_questions}") | ||
| print(f"Self-consistency: {sc_correct}/{n_questions} correct ({100*sc_correct/n_questions:.1f}%), {sc_no_consensus} NO_CONSENSUS") | ||
| print(f"Graph-of-Thought: {got_correct}/{n_questions} correct ({100*got_correct/n_questions:.1f}%), {got_no_consensus} NO_CONSENSUS") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Aggregate stats use n_questions instead of len(rows).
If resuming with a smaller n_questions than a previously completed run (already_done >= n_questions), the main loop never executes, but the aggregate section still divides by n_questions while rows may contain more entries, producing incorrect percentages/denominators.
🔧 Suggested fix
- sc_correct = sum(r["sc_correct"] for r in rows)
- got_correct = sum(r["got_correct"] for r in rows)
- sc_no_consensus = sum(r["sc_pred"] is None for r in rows)
- got_no_consensus = sum(r["got_pred"] is None for r in rows)
-
- print("\n=== Results ===")
- print(f"N = {n_questions}")
- print(f"Self-consistency: {sc_correct}/{n_questions} correct ({100*sc_correct/n_questions:.1f}%), {sc_no_consensus} NO_CONSENSUS")
- print(f"Graph-of-Thought: {got_correct}/{n_questions} correct ({100*got_correct/n_questions:.1f}%), {got_no_consensus} NO_CONSENSUS")
+ total = len(rows)
+ sc_correct = sum(r["sc_correct"] for r in rows)
+ got_correct = sum(r["got_correct"] for r in rows)
+ sc_no_consensus = sum(r["sc_pred"] is None for r in rows)
+ got_no_consensus = sum(r["got_pred"] is None for r in rows)
+
+ print("\n=== Results ===")
+ print(f"N = {total}")
+ print(f"Self-consistency: {sc_correct}/{total} correct ({100*sc_correct/total:.1f}%), {sc_no_consensus} NO_CONSENSUS")
+ print(f"Graph-of-Thought: {got_correct}/{total} correct ({100*got_correct/total:.1f}%), {got_no_consensus} NO_CONSENSUS")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| sc_correct = sum(r["sc_correct"] for r in rows) | |
| got_correct = sum(r["got_correct"] for r in rows) | |
| sc_no_consensus = sum(r["sc_pred"] is None for r in rows) | |
| got_no_consensus = sum(r["got_pred"] is None for r in rows) | |
| print("\n=== Results ===") | |
| print(f"N = {n_questions}") | |
| print(f"Self-consistency: {sc_correct}/{n_questions} correct ({100*sc_correct/n_questions:.1f}%), {sc_no_consensus} NO_CONSENSUS") | |
| print(f"Graph-of-Thought: {got_correct}/{n_questions} correct ({100*got_correct/n_questions:.1f}%), {got_no_consensus} NO_CONSENSUS") | |
| total = len(rows) | |
| sc_correct = sum(r["sc_correct"] for r in rows) | |
| got_correct = sum(r["got_correct"] for r in rows) | |
| sc_no_consensus = sum(r["sc_pred"] is None for r in rows) | |
| got_no_consensus = sum(r["got_pred"] is None for r in rows) | |
| print("\n=== Results ===") | |
| print(f"N = {total}") | |
| print(f"Self-consistency: {sc_correct}/{total} correct ({100*sc_correct/total:.1f}%), {sc_no_consensus} NO_CONSENSUS") | |
| print(f"Graph-of-Thought: {got_correct}/{total} correct ({100*got_correct/total:.1f}%), {got_no_consensus} NO_CONSENSUS") |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@benchmark_gsm8k.py` around lines 190 - 198, The aggregate summary in the
results block uses n_questions as the denominator even though the actual
evaluated data is in rows, which can become stale when resuming with fewer
questions than an existing run. Update the summary logic near the results
printout in benchmark_gsm8k.py to compute totals and percentages from len(rows)
(or otherwise clamp to the evaluated row count) instead of n_questions, and keep
the displayed N consistent with the actual number of processed rows. Use the
existing variables sc_correct, got_correct, sc_no_consensus, got_no_consensus,
and rows to locate the fix.
| // Per-call ggml graph node budget. Sized to comfortably fit MoE + spec | ||
| // decode batches (every layer's MoE op count is ~10-20 nodes). | ||
| inline constexpr std::size_t GRAPH_MAX_NODES = 1ull << 19; | ||
| inline constexpr std::size_t GRAPH_MAX_NODES = 1ull << 14; // reduced from 1<<19 (524288) to 1<<14 (16384) — the original value requests a ~10GB scheduler buffer regardless of model size, which fails malloc on memory-constrained systems for small models like Qwen3-0.6B. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n 'GRAPH_MAX_NODES' driver/portable -A3 -B3Repository: pie-project/pie
Length of output: 5255
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== graph_common.hpp =="
cat -n driver/portable/src/graph_common.hpp | sed -n '1,120p'
echo
echo "== graph_qwen3.cpp around node estimates =="
sed -n '1,130p' driver/portable/src/graph_qwen3.cpp | cat -n
echo
echo "== graph_qwen3_5.cpp around node estimates =="
sed -n '330,430p' driver/portable/src/graph_qwen3_5.cpp | cat -n
echo
echo "== executor.cpp scheduler/init usage =="
sed -n '400,460p' driver/portable/src/executor/executor.cpp | cat -n
sed -n '1128,1155p' driver/portable/src/executor/executor.cpp | cat -nRepository: pie-project/pie
Length of output: 24644
🌐 Web query:
ggml graph node capacity exceeded behavior ggml_new_graph_custom ggml_backend_sched_new graph max nodes
💡 Result:
When the node capacity of a ggml_cgraph is exceeded, the behavior depends on the specific implementation, but it typically results in memory corruption, silent failures, or crashes because ggml_cgraph structures are often backed by fixed-size buffers [1][2]. Key technical details regarding capacity and management include: 1. Understanding Capacity: The maximum number of nodes is governed by constants like GGML_DEFAULT_GRAPH_SIZE (typically 2048) [3][4][5]. When constructing graphs that exceed this, you must use ggml_new_graph_custom (or similar APIs that accept an n_nodes_max parameter) to explicitly allocate sufficient space [3][4]. 2. Backend-Specific Risks: Exceeding capacity can cause significant issues in specialized backends (such as CUDA) [1]. Some backends maintain their own internal ring buffers or metadata structures indexed by these limits (e.g., GGML_CUDA_MAX_NODES). If a graph exceeds the capacity managed by these internal buffers, the backend may silently overwrite data or trigger crashes at unpredictable points during execution, as it essentially runs out of "extra" slots for tensors [1]. 3. Scheduler Requirements: When using ggml_backend_sched_new, the graph_size parameter defines the capacity for the scheduler's internal management [5][6]. The scheduler uses this value to pre-allocate buffers [5]. If you attempt to compute a graph with a different topology or larger memory requirements than what was reserved, it can trigger reallocations, synchronization overhead, or error messages, though the scheduler is designed to handle common use cases through heuristics rather than complex dynamic optimization [5][7]. 4. Best Practices: To avoid capacity issues, it is recommended to: - Pre-calculate the required size using ggml_graph_overhead_custom before allocating the graph context [8]. - Use ggml_backend_sched_reserve with a "measure" graph that represents the maximum expected workload (e.g., maximum transformer context size) to ensure buffers are sized correctly for subsequent operations [5][7]. - If you frequently hit these limits, evaluate whether the computation graph can be simplified or modularized to reduce the total node count, rather than simply increasing the capacity constant [2].
Citations:
- 1: Bug in CUDA backend when allocated tensors in a given buffer exceed GGML_CUDA_MAX_NODES ggml-org/ggml#651
- 2: Initialize ggml_cgraph on heap ggml-org/ggml#251
- 3: https://ggml-org-ggml.mintlify.app/concepts/computation-graph
- 4: https://xsxszab.github.io/posts/ggml-deep-dive-iii/
- 5: https://ggml-org-ggml.mintlify.app/backends/overview
- 6: https://rdrr.io/cran/ggmlR/man/ggml_backend_sched_new.html
- 7: Can someone explain what ggml_backend_sched_t do? ggml-org/llama.cpp#10182
- 8: https://ggml-org-ggml.mintlify.app/api/context
GRAPH_MAX_NODES is too small for supported large batches
graph_qwen3.cpp already estimates ~150k nodes for 512 requests over 28 layers, so 1 << 14 will under-size the graph budget on supported Qwen3 paths. Since the same constant also sizes the backend scheduler buffer, this risks graph-build/runtime failure on larger configs; keep a much larger cap or make the budget model/batch-specific.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@driver/portable/src/graph_common.hpp` around lines 22 - 24, GRAPH_MAX_NODES
is now too low for supported Qwen3 large-batch paths and can under-size both
graph construction and the backend scheduler buffer. Update the constant in
graph_common.hpp to preserve a much larger headroom, or refactor the budget to
be model/batch-specific using the existing graph_qwen3.cpp sizing logic so large
request counts do not fail at runtime.
| fn extract_final_answer(text: &str) -> Option<f64> { | ||
| let cleaned = strip_think_blocks(text); | ||
| let lower = cleaned.to_lowercase(); | ||
| let marker = "answer:"; | ||
| let idx = lower.rfind(marker)?; | ||
| let after = &cleaned[idx + marker.len()..]; | ||
|
|
||
| let first_line = after.lines().next().unwrap_or("").trim(); | ||
| let numeric: String = first_line | ||
| .chars() | ||
| .filter(|c| c.is_ascii_digit() || *c == '.' || *c == '-') | ||
| .collect(); | ||
|
|
||
| if numeric.is_empty() { | ||
| None | ||
| } else { | ||
| numeric.parse::<f64>().ok() | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does Rust String::to_lowercase change the byte length of a string for some Unicode characters?
💡 Result:
Yes, Rust's String::to_lowercase method can change the byte length of a string [1]. Because Unicode case mapping is complex, some characters can expand into multiple characters when converted to lowercase [1][2]. Since Rust strings are UTF-8 encoded, and different characters require a varying number of bytes to store (from 1 to 4 bytes per Unicode scalar value), an increase in the number of characters—or a replacement of a character with one that has a different UTF-8 byte representation—will result in a change to the total byte length of the string [2][3]. For example, while case conversion typically maintains a 1:1 mapping, it is not guaranteed. This is why String::to_lowercase returns a new String rather than modifying the original one in-place, as the new string may require a different amount of memory [1]. If you need to perform case conversion without changing byte length or without allocating a new string, the standard library provides methods like str::to_ascii_lowercase, which is restricted to ASCII characters and preserves the byte length, as it only maps 'A'-'Z' to 'a'-'z' [1][4].
Citations:
- 1: https://dev-doc.rust-lang.org/alloc/string/struct.String.html
- 2: https://unicodefyi.com/guide/unicode-in-rust/
- 3: https://doc.rust-lang.org/stable/book/ch08-02-strings.html?highlight=String
- 4: Separate to_lowercase() into correct Unicode and simple implementations rust-lang/rust#26244
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== inferlets/graph-of-thought/src/lib.rs =="
sed -n '60,105p' inferlets/graph-of-thought/src/lib.rs
echo
echo "== inferlets/self-consistency/src/lib.rs =="
sed -n '60,105p' inferlets/self-consistency/src/lib.rs
echo
echo "== byte-length probe for Rust lowercasing edge cases =="
python3 - <<'PY'
samples = ["İ", "ẞ", "Σ", "ff", "A", "é", "ß"]
for s in samples:
lower = s.lower()
print(repr(s), len(s.encode()), "->", repr(lower), len(lower.encode()))
PYRepository: pie-project/pie
Length of output: 3208
Use the lowercase buffer for the slice
idx comes from lower, but after is sliced from cleaned. to_lowercase() can change byte length for Unicode text, so this can hit the wrong boundary or panic; apply the same fix in inferlets/self-consistency/src/lib.rs as well.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@inferlets/graph-of-thought/src/lib.rs` around lines 74 - 92,
`extract_final_answer` is slicing from `cleaned` using an index found in the
lowercased buffer, which can misalign for Unicode because `to_lowercase()` may
change byte length. Update `extract_final_answer` to derive the substring from
the same lowercase string used for `rfind`, or otherwise map the index safely
before slicing, and make the same correction in the matching
`extract_final_answer` implementation in
`inferlets/self-consistency/src/lib.rs`.
| /// Majority vote over extracted answers. Ties broken by first-occurrence | ||
| /// order among the tied values. Entries with no parseable answer are | ||
| /// excluded from voting entirely. | ||
| fn majority_vote(answers: &[Option<f64>]) -> Option<f64> { | ||
| let mut counts: Vec<(f64, usize)> = Vec::new(); | ||
|
|
||
| for ans in answers.iter().flatten() { | ||
| if let Some(entry) = counts.iter_mut().find(|(v, _)| (*v - *ans).abs() < 1e-9) { | ||
| entry.1 += 1; | ||
| } else { | ||
| counts.push((*ans, 1)); | ||
| } | ||
| } | ||
|
|
||
| counts | ||
| .into_iter() | ||
| .max_by_key(|(_, count)| *count) | ||
| .map(|(v, _)| v) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the surrounding code in inferlets/graph-of-thought/src/lib.rs
file="inferlets/graph-of-thought/src/lib.rs"
wc -l "$file"
sed -n '1,180p' "$file"
# Probe Rust tie behavior locally with a minimal equivalent example in Python.
python3 - <<'PY'
counts = [(1.0, 2), (2.0, 2), (3.0, 1)]
# Python's max would keep first; Rust's max_by_key keeps the last maximal element.
# Print the equivalent expected tie behavior for clarity.
print("max(counts, key=count) in Python returns first maximal element:", max(counts, key=lambda x: x[1]))
PYRepository: pie-project/pie
Length of output: 6838
🌐 Web query:
Rust Iterator::max_by_key tie-breaking behavior last maximal element documentation
💡 Result:
In Rust, the Iterator::max_by_key method returns the last element that produces the maximum key when multiple elements are considered equal [1][2]. If the comparison determines that two or more elements are tied for the maximum value, the method effectively picks the one that appears latest in the iteration sequence [1][3]. It is important to distinguish this from the standalone std::cmp::max_by_key function, which compares only two values and returns the second argument if they are equal [4][5][6]. The Iterator method's behavior of returning the "last" maximal element is a result of its internal implementation, which keeps track of the "best" element found so far and updates it only when a strictly greater key is encountered [3].
Citations:
- 1: https://rustbites.com/posts/bite-179/
- 2: https://docs.rs/iter-first-max/latest/iter_first_max/
- 3: https://doc.rust-lang.org/1.14.0/std/iter/trait.Iterator.html
- 4: https://doc.rust-lang.org/std/cmp/fn.max_by_key.html
- 5: https://doc.rust-lang.org/stable/std/cmp/fn.max_by_key.html
- 6: https://doc.rust-lang.org/stable/core/cmp/fn.max_by_key.html
Tie-break doc is reversed. max_by_key keeps the last maximal entry, so tied answers currently resolve to last-occurrence order, not first-occurrence. Update the doc or make the tie-break explicit if first-seen order is required.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@inferlets/graph-of-thought/src/lib.rs` around lines 94 - 112, The tie-break
behavior in majority_vote is documented as first-occurrence, but max_by_key will
pick the last maximal entry among ties. Update majority_vote so the tie-break is
explicit and matches the intended first-seen order, or adjust the doc comment to
reflect the actual last-occurrence behavior; keep the change localized to
majority_vote and its comment.
|
|
||
| [parameters] | ||
| question = {type = "string", optional = true, description = "The question to answer"} | ||
| proposal_tokens = {type = "string", optional = true, description = "Max tokens per proposal sample; array length = number of proposals (must be even)"} |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate parameter-type validation/enforcement logic in the runtime and bakery tooling
rg -n -C4 'ParameterType' runtime/src/program/manifest.rs
rg -n -C4 "manifest.get\\(.parameters.\\)|param_type|ParameterType" sdk/tools/bakery/src/bakery/inferlet.pyRepository: pie-project/pie
Length of output: 2222
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Pie.toml ==\n'
cat -n inferlets/self-consistency/Pie.toml | sed -n '1,80p'
printf '\n== inferlets/self-consistency/src/lib.rs ==\n'
cat -n inferlets/self-consistency/src/lib.rs | sed -n '1,120p'
printf '\n== manifest schema/runtime validation ==\n'
rg -n -C4 'ParameterType|Parameter|parameters|validate' runtime/src/program/manifest.rs runtime/src/program -g '!**/target/**'
printf '\n== bakery inferlet param handling ==\n'
rg -n -C4 'param_type|parameters|type = "string"|type = "int"|type = "float"|type = "bool"|description' sdk/tools/bakery/src/bakery/inferlet.py sdk/tools/bakery -g '!**/target/**'Repository: pie-project/pie
Length of output: 41035
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== manifest parsing / schema docs ==\n'
rg -n -C3 'type = "string"|type = "int"|type = "float"|type = "bool"|array|list|ParameterType|parameters' README.md docs runtime inferlets sdk/tools/bakery -g '!**/target/**'
printf '\n== inferlet macro / input payload path ==\n'
rg -n -C4 'serde_json|Deserialize|Input|parameters' inferlet inferlets runtime sdk -g '!**/target/**'
printf '\n== exact self-consistency use of proposal_tokens ==\n'
rg -n -C4 'proposal_tokens' inferlets/self-consistency/src/lib.rs inferlets/self-consistency/Pie.tomlRepository: pie-project/pie
Length of output: 50374
proposal_tokens needs a list-capable schema, not string. inferlets/self-consistency/Pie.toml declares a scalar field, but inferlets/self-consistency/src/lib.rs deserializes it as Vec<usize>. That makes manifest-driven tooling emit the wrong input shape for a value that must be a JSON array. Extend the manifest schema to support arrays, or change the inferlet to accept a scalar encoding.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@inferlets/self-consistency/Pie.toml` at line 13, The manifest schema for
proposal_tokens is declared as a scalar string, but
inferlets/self-consistency/src/lib.rs expects a Vec<usize>, so update the
Pie.toml declaration to a list-capable array schema for proposal_tokens or align
the inferlet deserialization to a scalar format. Use the proposal_tokens field
definition in Pie.toml and the corresponding parsing logic in the
self-consistency lib as the matching points, and make sure the manifest-driven
input shape matches the actual JSON array expected by the inferlet.
| fn extract_final_answer(text: &str) -> Option<f64> { | ||
| let cleaned = strip_think_blocks(text); | ||
| let lower = cleaned.to_lowercase(); | ||
| let marker = "final answer:"; | ||
| let idx = lower.rfind(marker)?; | ||
| let after = &cleaned[idx + marker.len()..]; | ||
|
|
||
| let first_line = after.lines().next().unwrap_or("").trim(); | ||
| let numeric: String = first_line | ||
| .chars() | ||
| .filter(|c| c.is_ascii_digit() || *c == '.' || *c == '-') | ||
| .collect(); | ||
|
|
||
| if numeric.is_empty() { | ||
| None | ||
| } else { | ||
| numeric.parse::<f64>().ok() | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the target file and inspect the relevant section.
git ls-files inferlets/self-consistency/src/lib.rs
wc -l inferlets/self-consistency/src/lib.rs
cat -n inferlets/self-consistency/src/lib.rs | sed -n '1,140p'
# Read-only probe: reproduce the byte-offset behavior of to_lowercase vs original string.
python3 - <<'PY'
samples = [
"İ before final answer: 42",
"abc FINAL ANSWER: 42",
"foo final answer: 1",
"xİy final answer: 3.14",
]
marker = "final answer:"
for s in samples:
cleaned = s
lower = cleaned.to_lowercase() if False else cleaned.lower()
idx = lower.rfind(marker)
print("\nINPUT:", repr(s))
print("cleaned bytes:", len(cleaned.encode()))
print("lower bytes:", len(lower.encode()))
print("idx from lower:", idx)
if idx != -1:
start = idx + len(marker)
print("slice start:", start)
try:
print("cleaned slice:", cleaned[start:])
except Exception as e:
print("slice error:", type(e).__name__, e)
PYRepository: pie-project/pie
Length of output: 6210
🏁 Script executed:
python3 - <<'PY'
def rust_like_slice_ok(original: str):
cleaned = original
lower = cleaned.lower()
marker = "final answer:"
idx = lower.encode("utf-8").find(marker.encode("ascii"))
if idx < 0:
return None
start = idx + len(marker)
b = cleaned.encode("utf-8")
# valid UTF-8 char boundary positions in Rust are byte positions at codepoint starts or end
boundaries = {0}
i = 0
while i < len(b):
ch = b[i]
if ch < 0x80:
step = 1
elif (ch & 0xE0) == 0xC0:
step = 2
elif (ch & 0xF0) == 0xE0:
step = 3
elif (ch & 0xF8) == 0xF0:
step = 4
else:
raise ValueError("invalid utf-8")
i += step
boundaries.add(i)
return {
"cleaned_bytes": len(b),
"lower_bytes": len(lower.encode("utf-8")),
"idx": idx,
"start": start,
"is_boundary": start in boundaries,
"slice": None if start > len(b) else b[start:].decode("utf-8", "replace"),
}
samples = [
"İ before final answer: 42",
"xİy final answer: 3.14",
"ß final answer: 7",
"Hello final answer: 1",
"Ａ final answer: 2", # fullwidth A; lower doesn't expand
]
for s in samples:
print("\nINPUT:", repr(s))
print(rust_like_slice_ok(s))
PYRepository: pie-project/pie
Length of output: 813
🏁 Script executed:
python3 - <<'PY'
def rust_like_slice_ok(original: str):
cleaned = original
lower = cleaned.lower()
marker = "final answer:"
idx = lower.encode("utf-8").find(marker.encode("ascii"))
if idx < 0:
return None
start = idx + len(marker)
b = cleaned.encode("utf-8")
boundaries = {0}
i = 0
while i < len(b):
ch = b[i]
if ch < 0x80:
step = 1
elif (ch & 0xE0) == 0xC0:
step = 2
elif (ch & 0xF0) == 0xE0:
step = 3
elif (ch & 0xF8) == 0xF0:
step = 4
else:
raise ValueError("invalid utf-8")
i += step
boundaries.add(i)
return {
"cleaned_bytes": len(b),
"lower_bytes": len(lower.encode("utf-8")),
"idx": idx,
"start": start,
"is_boundary": start in boundaries,
"slice": None if start > len(b) else b[start:].decode("utf-8", "replace"),
}
samples = [
"İ before final answer: 42",
"xİy final answer: 3.14",
"ß final answer: 7",
"Hello final answer: 1",
"Ａ final answer: 2",
]
for s in samples:
print("\nINPUT:", repr(s))
print(rust_like_slice_ok(s))
PYRepository: pie-project/pie
Length of output: 813
🏁 Script executed:
python3 - <<'PY'
# Search for a minimal counterexample where:
# idx = lower.rfind("final answer:")
# start = idx + len("final answer:")
# is not a valid slice start in the original cleaned string.
#
# We approximate Rust's Unicode lowercase behavior with Python's .lower(),
# then validate against UTF-8 byte boundaries.
marker = "final answer:"
candidates = ["İ", "ẞ", "ʼn", "Dž", "Σ", "Μ", "ϴ", "K", "ᾈ", "ᾼ"]
def byte_boundaries(s: str):
b = s.encode("utf-8")
out = {0}
i = 0
while i < len(b):
c = b[i]
if c < 0x80:
step = 1
elif (c & 0xE0) == 0xC0:
step = 2
elif (c & 0xF0) == 0xE0:
step = 3
else:
step = 4
i += step
out.add(i)
return len(b), out
for ch in candidates:
for suffix in ["", " ", " 1", " 42"]:
cleaned = f"{ch}{suffix}{marker}"
lower = cleaned.lower()
idx = lower.rfind(marker)
start = idx + len(marker)
clen, boundaries = byte_boundaries(cleaned)
ok = start in boundaries and start <= clen
print(repr(cleaned), "->", {"cleaned_bytes": clen, "lower_bytes": len(lower.encode()), "idx": idx, "start": start, "valid": ok})
PYRepository: pie-project/pie
Length of output: 4173
🏁 Script executed:
cat <<'RS' >/tmp/check.rs
fn main() {
let samples = [
"İ before final answer: 42",
"xİy final answer: 3.14",
"ß final answer: 7",
"Hello final answer: 1",
"Ａ final answer: 2",
];
let marker = "final answer:";
for s in samples {
let lower = s.to_lowercase();
let idx = lower.rfind(marker);
println!("{:?}", s);
println!(" cleaned bytes = {}", s.len());
println!(" lower bytes = {}", lower.len());
println!(" idx = {:?}", idx);
if let Some(i) = idx {
let start = i + marker.len();
print!(" slice start = {}", start);
println!(" boundary? = {}", s.is_char_boundary(start));
if s.is_char_boundary(start) {
println!(" tail = {:?}", &s[start..]);
}
}
}
}
RS
rustc /tmp/check.rs -O -o /tmp/check && /tmp/checkRepository: pie-project/pie
Length of output: 400
🏁 Script executed:
python3 - <<'PY'
marker = b"final answer:"
def utf8_boundaries(b):
out = {0}
i = 0
while i < len(b):
c = b[i]
if c < 0x80:
step = 1
elif (c & 0xE0) == 0xC0:
step = 2
elif (c & 0xF0) == 0xE0:
step = 3
else:
step = 4
i += step
out.add(i)
return out
# Find characters whose lowercase mapping changes UTF-8 byte length.
hits = []
for cp in range(0x110000):
ch = chr(cp)
low = ch.lower()
if len(low.encode("utf-8")) != len(ch.encode("utf-8")):
hits.append((ch, ch.encode("utf-8"), low.encode("utf-8")))
if len(hits) >= 20:
break
print("length-changing lowercase samples:")
for ch, ob, lb in hits:
print(repr(ch), len(ob), "->", len(lb), repr(ch.lower()))
print("\nCounterexample probes with marker after a single expanding char:")
for ch, ob, lb in hits[:20]:
s = ch + "final answer:"
b = s.encode("utf-8")
lower_b = s.lower().encode("utf-8")
idx = lower_b.rfind(marker)
start = idx + len(marker)
print(
repr(s),
{"orig_len": len(b), "lower_len": len(lower_b), "idx": idx, "start": start,
"boundary": start in utf8_boundaries(b), "slice_ok": start <= len(b) and start in utf8_boundaries(b)}
)
PYRepository: pie-project/pie
Length of output: 2761
Avoid slicing cleaned with an offset from to_lowercase()
rfind() returns a byte index in the lowercased buffer, but Unicode lowercasing can change byte length before the marker, making cleaned[idx + marker.len()..] panic on some inputs. Use a byte-preserving ASCII lowercase/search or a case-insensitive search on the original string.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@inferlets/self-consistency/src/lib.rs` around lines 67 - 85,
`extract_final_answer` is slicing `cleaned` using an index computed from
`to_lowercase()`, which can become invalid for Unicode text; update the marker
search to use a byte-preserving approach or a case-insensitive search on the
original string, and then derive the `after` slice from the same source string
so `rfind`/slice offsets stay aligned.
- Add Generator::collect_text_with_tokens() to the SDK, returning exact
token counts alongside decoded text (no functional change to existing
collect_text()).
- Update self-consistency and graph-of-thought inferlets to use it,
returning {"answer": ..., "tokens": N} instead of a bare string so
token cost can be tracked per method.
- Add a single-shot baseline mode to benchmark_gsm8k.py (one proposal at
the full matched token budget), reusing the self-consistency inferlet.
- Update parse_result()/summarize() to handle the new JSON format and
report average tokens/question per method.
Results (N=50, 11 excluded due to WSL2/CUDA infra crashes, 39 valid):
- Baseline (single-shot): 23/39 correct (59.0%), avg ~1,007 tokens/question
- Self-consistency: 18/39 correct (46.2%), avg ~3,208 tokens/question
- Graph-of-Thought: 7/39 correct (17.9%), avg ~3,466 tokens/question
Baseline outperforms both multi-proposal methods while using roughly a
third of the tokens, on GSM8K at this model scale (Qwen3-0.6B).
There was a problem hiding this comment.
🧹 Nitpick comments (2)
benchmark_gsm8k.py (1)
219-219: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDead
predsvariable insummarize.
preds = [r for r in rows if ... or True]— theor Truemakes the condition always true, andpredsis never used afterward. Remove it.♻️ Proposed fix
def summarize(key, label): - preds = [r for r in rows if r[f"{key}_pred"] is not None or r[f"{key}_tokens"] is not None or True] correct = sum(r[f"{key}_correct"] for r in rows)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benchmark_gsm8k.py` at line 219, The summarize logic has a dead preds assignment because the filter condition is always true due to the trailing or True, and the preds result is never used afterward. In summarize, remove the pointless preds computation entirely, or if the rows need filtering, replace the always-true condition with the intended predicate using key_pred and key_tokens so the variable has a meaningful purpose.benchmark_results.json (1)
1-652: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRecorded benchmark output committed as a data file.
This large auto-generated results snapshot (with sibling
benchmark_results_sc_vs_got_only.jsonandbenchmark_results_test_n3.json) is committed directly to the repo. Sincebenchmark_gsm8k.pywrites to this same path incrementally on every run, future runs will produce diffs/merge noise here. Consider moving generated results under a gitignored output directory, or clearly documenting these as frozen point-in-time artifacts for the PR write-up rather than live state.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benchmark_results.json` around lines 1 - 652, The benchmark results JSON is being treated as a tracked source file even though `benchmark_gsm8k.py` continuously rewrites it, which will cause noisy diffs on future runs. Move the generated artifacts such as `benchmark_results.json`, `benchmark_results_sc_vs_got_only.json`, and `benchmark_results_test_n3.json` to a gitignored output location, or otherwise make it explicit in the benchmark-writing code that these are frozen snapshots. Update the code path in `benchmark_gsm8k.py` that emits the results so the repo does not accumulate incremental benchmark output changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@benchmark_gsm8k.py`:
- Line 219: The summarize logic has a dead preds assignment because the filter
condition is always true due to the trailing or True, and the preds result is
never used afterward. In summarize, remove the pointless preds computation
entirely, or if the rows need filtering, replace the always-true condition with
the intended predicate using key_pred and key_tokens so the variable has a
meaningful purpose.
In `@benchmark_results.json`:
- Around line 1-652: The benchmark results JSON is being treated as a tracked
source file even though `benchmark_gsm8k.py` continuously rewrites it, which
will cause noisy diffs on future runs. Move the generated artifacts such as
`benchmark_results.json`, `benchmark_results_sc_vs_got_only.json`, and
`benchmark_results_test_n3.json` to a gitignored output location, or otherwise
make it explicit in the benchmark-writing code that these are frozen snapshots.
Update the code path in `benchmark_gsm8k.py` that emits the results so the repo
does not accumulate incremental benchmark output changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8e7fc280-360f-4cf4-a036-1ecb1cf33778
📒 Files selected for processing (7)
benchmark_gsm8k.pybenchmark_results.jsonbenchmark_results_sc_vs_got_only.jsonbenchmark_results_test_n3.jsoninferlets/graph-of-thought/src/lib.rsinferlets/self-consistency/src/lib.rssdk/rust/inferlet/src/generation.rs
✅ Files skipped from review due to trivial changes (2)
- benchmark_results_sc_vs_got_only.json
- benchmark_results_test_n3.json
🚧 Files skipped from review as they are similar to previous changes (1)
- inferlets/graph-of-thought/src/lib.rs
2 participants
Summary
Adds a self-consistency inferlet to compare against Graph-of-Thought (GoT), plus a crash-resilient benchmark harness to evaluate both on GSM8K at matched token budgets.
Changes
self-consistencyinferlet: generates N proposals in parallel (batched in chunks of 4 to reduce peak VRAM), extracts final answers, and takes a majority vote.graph-of-thought/src/lib.rsnow generates proposals in chunks of 4 instead of all-at-once, reducing peak concurrent KV cache usage on memory-constrained GPUs (tested on a 4GB RTX 2050).benchmark_gsm8k.py: crash-resilient harness that runs both inferlets on GSM8K questions at matched ~3,584 token budgets, saves results incrementally (survives server crashes/restarts), and auto-restartspie serveon failure.analyze_results.py/install_inferlets.py: supporting scripts for result analysis and inferlet installation.gsm8k_test.jsonl: GSM8K test split used for the benchmark.Results (N=50, matched ~3,584 token budget)
12/50 questions failed due to WSL2/CUDA infrastructure crashes (unrelated to model behavior) and are excluded from accuracy figures below.
Self-consistency outperforms GoT by ~2x on this benchmark. Notably, GoT's shortfall is almost entirely a non-convergence problem rather than wrong answers (1 wrong vs 28 NO_CONSENSUS) — still investigating whether this is an extraction/parsing issue in the aggregation output or genuine reasoning degradation during pairwise aggregation at this model scale (Qwen3-0.6B).
Known limitations
dxgvmb_send_wait_sync_object_gpustalls unrelated to this code; harness auto-recovers via retry logic but 12/50 questions were still lost to repeated crashes.Summary by CodeRabbit