allenai / allenai/asta-autodiscovery
Validate that `n` actually delivers independent samples for our primary models
- Ngôn ngữ chính
- Python
- Star
- 10
- Fork
- 2
- Merge trung bình
- 1 ngày 21 giờ
- Pull request đã merge (30 ngày)
- 11
Mô tả
Split out of #68 review, where the `n` parameter came up as the one non-standard thing we ask of every provider. Related: #67, #71 (changes the default model), and a Slack thread with @radamson.
## Why this needs its own investigation
`n` is load-bearing: `--n_belief_samples` (default 5) becomes `n` on a single request, and belief distributions are built from those samples. The framing so far has been *"does the provider support `n`?"* — but measuring it turned up three failure modes that are all quieter and more consequential than outright rejection:
1. A provider can accept `n`, return HTTP 200, and give back **fewer choices than asked** — no error.
2. A provider can return the requested count where the samples are **not independent** — duplicate candidates, so belief variance is understated with nothing to detect it.
3. Our per-provider `n` cap is a hardcoded guess that is **wrong in both directions** depending on the model.
None of these are caught today: `query_llm` never compares `len(response.choices)` to the requested `batch_n`, and the usage record stores the *requested* count (`metadata["n"] = batch_n`) while its own docstring claims it is *"the actual per-request sample count"*. So the one artifact you would check afterwards confirms the wrong number.
## The primary-model list is the blocker
The set of models this should be validated against is in flux:
- **#71** switches all three of `--model` / `--belief_model` / `--vision_model` to `gemini-3.7-flash`.
- The Copilot model set needs input from the collaborator driving that work — Copilot's own `/models` advertises models that are not callable, so "supported" there is account-dependent (see #68 for the detail).
**Action: settle the primary-model list first**, then run the matrix below against it. Everything measured so far is groundwork, not a conclusion.
## Measurements so far
All via `autodiscovery.llm.complete` on this branch (litellm transport), prompt = *"pick a random integer between 1 and 1000000"*, `temperature=1.0` where accepted. `distinct` counts unique values in one response.
### Independence — the finding that matters most
| Model | n=3 | n=5 | n=8 |
| --- | --- | --- | --- |
| `vertex_ai/gemini-3.7-flash` ← **#71's incoming default** | — | 5 → **2 distinct** | 8 → **3 distinct** |
| `vertex_ai/gemini-3.1-pro-preview` ← current default | 3 → **2** | 5 → **3** | 8 → 7 |
| `vertex_ai/gemini-3-flash-preview` | 3 → 3 | 5 → 5 | 8 → 8 |
| `openai/gpt-4o`, `openai/gpt-5`, `openai/gpt-5-mini` | — | 5 → 5 | 10 → 10 |
| `github_copilot/gpt-5.4` | — | 5 → 5 | — |
Collisions in a 1-in-a-million range are not chance. **`gemini-3.7-flash` is the worst of the set** — 2 distinct values out of 5 — and #71 makes it the model for all three roles. `gemini-3-flash-preview` is clean, so this is not "all Gemini"; it is model-specific and needs measuring per candidate default.
Worth confirming with a belief-shaped prompt rather than a random integer before drawing conclusions — the effect may be smaller when the output space is genuinely constrained. That is the first task here.
### Provider `n` ceilings vs. our hardcoded caps
`llm.max_n` currently returns: `vertex_ai` → 5, `openai` + reasoning → 8, everything else → `None`.
| Provider | Real ceiling (measured) | Our cap | Consequence |
| --- | --- | --- | --- |
| `vertex_ai` (3.1-pro, 3-flash, 3.7-flash) | **8** (`n=9` → `BadRequestError`) | 5 | Over-conservative: batches at n=6-8 where one call would do. Correct, just extra round-trips. |
| `openai` | ≥10 | 8 (reasoning) / None | Fine. |
| `github_copilot` | **8** (`Invalid 'n': ... Expected a value <= 8, but got 10`) | `None` | **Broken above 8** — see below. |
### A regression #68 introduced for Copilot
On `main`, Copilot never sent `n`. `copilot_provider` looped client-side:
```python
responses = [_sample()]
if n_samples > 1:
with ThreadPoolExecutor(...) as executor:
responses.extend(executor.map(lambda _: _sample(), range(n_samples - 1)))
```
That worked for any `n_samples`. #68 routes Copilot through litellm with `n=n_samples` in one call, and `max_n` returns `None` for Copilot, so nothing batches:
```
copilot/gpt-5.4 query_llm n=5 → 5 samples, 5 distinct OK
copilot/gpt-5.4 query_llm n=10 → BadRequestError: Invalid 'n': integer above maximum
value. Expected a value <= 8, but got 10 instead.
```
Latent at the default `n_belief_samples=5`. The fix is one line — `max_n` returning 8 for `github_copilot` so existing batching handles it — deliberately left out of #68 to keep that PR scoped to the Gemini defaults, which are unaffected.
### What #68 did *not* break
Worth stating, since swapping Vertex off its OpenAI-compatible endpoint was the plausible regression: `n` still works end to end for the Gemini defaults. litellm's native Vertex path maps `n` → `candidateCount`, and the 5-cap batching still fires.
```
query_llm vertex_ai/gemini-3.1-pro-preview n=5 → 5 samples n=10 → 10 samples (2 calls)
query_llm vertex_ai/gemini-3-flash-preview n=5 → 5 samples n=10 → 10 samples (2 calls)
```
## Proposed scope
1. **Settle the primary-model list** (blocked on #71 and Copilot-collaborator input).
2. Re-run the independence matrix against that list using a **belief-shaped prompt**, not a random integer, to see whether the duplicate-candidate effect is material for the actual workload.
3. Decide whether `n` is the right mechanism at all for models with poor candidate independence, or whether belief sampling should issue separate requests — which is what the pre-#68 Copilot path effectively did.
4. Replace `max_n`'s hardcoded table with measured values, or derive it. Vertex is 5-should-be-8; Copilot is missing entirely.
5. Add a shortfall check: warn (or raise) when `len(choices) < batch_n`, and record the **delivered** count in `metadata["n"]` rather than the requested one. Fix the docstring either way.
## Note for #68 / #71 sequencing
These two PRs conflict semantically, not just textually. #71 sets the defaults to bare `gemini-3.7-flash`; #68 requires litellm-qualified names and rejects unqualified ones at startup. Whichever merges second needs `vertex_ai/gemini-3.7-flash`. They also overlap on `args.py`, `easy.py`, `run.py`, `agents.py`, `scripts/run_job.sh`, `standalone.md`, and `api/metrics/costs.py` (also touched by #70).
Hướng dẫn đóng góp
Chưa lập chỉ mục được hướng dẫn đóng góp cho kho mã nguồn này
Đánh giá
Issue này chưa được đánh giá.