Repository navigation
fix(assistantbench): disambiguate thousands-separator commas from decimal commas in fix_number - #414
Conversation
…imal commas in fix_number Closes ServiceNow#404 The old code called .replace(',', '.') unconditionally in fix_number(), causing two classes of silent scoring errors: - '3,080,000' -> '3.080.000' -> float() raises -> correct answer scores 0.0 - '1,000' -> '1.000' = 1.0 -> matched gold 1 -> wrong answer scores 1.0 Fix: extract disambiguation logic into _fix_comma() in evaluate_utils/utils.py and call it from both fix_number() implementations (evaluator.py and evaluate_dicts.py). Disambiguation rules: - Multiple commas -> US grouping, strip all ('3,080,000' -> '3080000') - Single comma + 3 digits -> thousands separator ('1,000' -> '1000') - Single comma + 1-2 digs -> European decimal comma ('14,2' -> '14.2') - Anything else -> leave unchanged (falls back to string comparison) Also narrows bare except: clauses to explicit exception types. Tests added (23 new cases): - test_fix_comma - 12-case parametrized unit test for _fix_comma - test_evaluate_comma_numbers - 9-case end-to-end question_scorer test - test_evaluate_comma_numbers_in_dicts - dict evaluation path - test_evaluate_comma_numbers_negative - negative thousands separator
|
Hi @Surya-5555, I've raised this PR to fix #404: AssistantBench's Could you please review it and approve the pending workflow run? Thank you, and have a great day! |
|
Hi @Vijaysrimari, thanks for the PR and for the clear description! The You can run the repo's formatter (see the code-format job / Once that's green, I'll review it. Thanks! |
…imal commas in fix_number Closes ServiceNow#404 The old code called .replace(',', '.') unconditionally in fix_number(), causing two classes of silent scoring errors: - '3,080,000' -> '3.080.000' -> float() raises -> correct answer scores 0.0 - '1,000' -> '1.000' = 1.0 -> matched gold 1 -> wrong answer scores 1.0 Fix: extract disambiguation logic into _fix_comma() in evaluate_utils/utils.py and call it from both fix_number() implementations (evaluator.py and evaluate_dicts.py). Disambiguation rules: - Multiple commas -> US grouping, strip all ('3,080,000' -> '3080000') - Single comma + 3 digits -> thousands separator ('1,000' -> '1000') - Single comma + 1-2 digs -> European decimal comma ('14,2' -> '14.2') - Anything else -> leave unchanged (falls back to string comparison) Also narrows bare except: clauses to explicit exception types. Tests added (23 new cases): - test_fix_comma - 12-case parametrized unit test for _fix_comma - test_evaluate_comma_numbers - 9-case end-to-end question_scorer test - test_evaluate_comma_numbers_in_dicts - dict evaluation path - test_evaluate_comma_numbers_negative - negative thousands separator
afae07b to
ccd59ee
Compare
|
Hi @Vijaysrimari, thanks for the clean PR and great tests! One edge case remains before merging: Approach to fix: Once that's added, happy to merge! |
…://github.com/Vijaysrimari/BrowserGym into fix/assistantbench-thousands-comma-scorer-404
Update regex from (-?\d+),(\d+) to (-?\d+),(\d+)(\.\d+)? so that strings like '1,000.50' and '1,234.56' are correctly recognised as thousands-separator notation and have the comma stripped, yielding '1000.50' and '1234.56' respectively. Add test cases for this edge case in test_fix_comma and test_evaluate_comma_numbers.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PR Description
Closes #404
What changed and why
fix_number()in bothevaluator.pyandevaluate_dicts.pypreviously called.replace(",", ".")unconditionally, which silently corrupted scores in two directions:"3,080,000"→"3.080.000"→float()raises → correct answer scores 0.0"1,000"→"1.000"=1.0→ matched gold1→ wrong answer scores 1.0The fix extracts the disambiguation logic into a single helper
_fix_comma(number: str) -> strin the sharedevaluate_utils/utils.py, then calls it from bothfix_number()implementations.Disambiguation rules (in priority order):
"3,080,000""3080000""1,000""1000""14,2""14.2""1,0000""1,0000""hello""hello"Files changed
evaluate_utils/utils.py_fix_comma()with full docstring — single source of truthevaluator.py.replace(",","."), imported and called_fix_comma; also narrowed bareexcept:toexcept (ValueError, TypeError)evaluate_utils/evaluate_dicts.pyfix_number()tests/assistantbench/test_evaluation.pyProof of work