Fix GPT-5 pricing: exact-model lookup shadowed by prefix fallback - #14
Merged
Conversation
…lback
resolve_genai_price_params checked startswith("gpt-5") before consulting
_GPT_MODEL_REF_MAP, so every GPT-5 variant was priced as base gpt-5. This
made published costs for gpt-5-mini-medium ~5x too high ($3.47 vs $0.69
actual for the zero-shot runs) and gpt-5-2-high ~29% too low ($4.99 vs
$6.99). Also corrects the gpt-5-pro-high map entry, which pointed at
base gpt-5 pricing.
Extraction scores are unaffected; only the cost_usd columns and the
cost/F1 pareto figure derive from this function.
Collaborator
|
this is a good catch thank you. I think we should raise for unknown models rather than falling back to gpt-5 pricing since it's highly unlikely for them to be the same cost. can you make that change (and update the test)? I should probably regenerate the pareto front cost chart bc of this bug |
Per PR feedback: an unknown model name is highly unlikely to cost the same as base gpt-5, so resolve_genai_price_params now raises ValueError instead of guessing (or returning None, which silently priced runs at $0). gpt-5-minimal/low/high get explicit map entries since they are valid config names that relied on the prefix fallback.
Contributor
Author
|
@curtischong thanks for the review, updated with requested feedback! |
Collaborator
|
Thank you so much for this! I'll update the charts later |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug
In
resolve_genai_price_params(scripts/paper/benchmarks/helpers/pricing.py), thestartswith("gpt-5")fallback runs before the_GPT_MODEL_REF_MAPlookup, making the map unreachable for everygpt-5*name — all GPT-5 variants get priced as basegpt-5. Separately, thegpt-5-pro-highmap entry points at"gpt-5"rather than"gpt-5-pro".Impact
Benchmark scores are unaffected — this function only computes
cost_usdafter a run. But the cost columns are off. Recomputing from the token counts already published incombined_results/zero_shot_run[1-3].csv:This flows into the cost/F1 comparison — e.g. at $0.69, gpt-5-mini-medium is cheaper than claude-haiku-4-5 ($1.72) rather than 2x more expensive.
Fix
_GPT_MODEL_REF_MAPbefore the prefix fallback (unknowngpt-5*names still fall back to basegpt-5)gpt-5-pro-highentrytests/test_pricing.pyasserting each model name resolves to its own price ref (skipped unless thepaperextra is installed, so the default test environment is unaffected)