fix(lm_eval): don't build continuous metrics with a null bound#206
Open
borgr wants to merge 1 commit into
Open
fix(lm_eval): don't build continuous metrics with a null bound#206borgr wants to merge 1 commit into
borgr wants to merge 1 commit into
Conversation
KNOWN_METRIC_BOUNDS declared ter as (0.0, None), and any metric absent from the table fell through to min/max = None. The adapter then built a continuous MetricConfig unconditionally, and the score-type validator raises "score_type 'continuous' requires max_score" at construction — so converting any lm-eval results containing ter (or an unlisted metric) crashed the conversion rather than producing output. - ter: use a finite 0-100 bound (the sacrebleu reporting scale, matching bleu) instead of None. - Skip a metric with no finite bound (e.g. perplexity) with a warning instead of constructing an invalid continuous config; EEE has no unbounded score_type. - Drop a task whose every metric was skipped rather than emit a contentless log. - Regression test covering ter + a skipped unbounded metric.
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
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.
The bug
The
lm_evalconverter crashes on results containingter— or any metric absent from its 15-entry bounds table.KNOWN_METRIC_BOUNDSdeclares'ter': (0.0, None), and for any metric not in the tableKNOWN_METRIC_BOUNDS.get(...)returnsNone. Inadapter.pythe bounds were then used to build acontinuousMetricConfigunconditionally:continuousrequires numericmin_score/max_score(enforced byvalidate_score_type_requirements), so with aNonebound theMetricConfig(...)constructor raisesscore_type 'continuous' requires max_score— the whole conversion fails. EEE has no unboundedscore_type, andfloat('inf')is not a fix (pydantic serializes it tonull, which fails the same validator on read-back, andInfinityis non-standard JSON).The fix
ter→(0.0, 100.0): TER (sacrebleu translation edit rate) is reported on a 0–100 scale, likebleuin the same table. It can exceed 100 in pathological cases, but 0–100 is the reported range and EEE does not range-check scores, so a finite nominal bound is correct here.perplexity,word_perplexity) with a warning, instead of constructing an invalidcontinuousconfig — since EEE cannot represent an unbounded metric.EvaluationLog.test_unbounded_metrics_do_not_produce_invalid_records):teryields a valid 0–100 continuous record; a genuinely-unbounded metric is skipped; a perplexity-only task is dropped.uv run pytest tests→ 207 passed, 20 skipped.ruff checkclean.