Uh oh!
There was an error while loading. Please reload this page.
Carry metric() to the next SI prefix when rounding reaches 1000 - #328
Conversation
metric() chose the SI prefix from floor(log10(value)) on the raw value and then rounded the mantissa to the requested precision. For a value just below a power-of-1000 boundary the rounded mantissa became 1000 while the prefix stayed put, so metric(999.9, "V") returned "1000 V" and metric(999999, "V") returned "1000 kV". That is exactly the "1230K"-style output the function's docstring promises to avoid. After rounding, if the mantissa reaches 1000 and a higher prefix is available (exponent < 30), advance to the next prefix bucket and re-divide. The top of the SI range (quetta) has no higher prefix, so it is left unchanged. Output for values not on a rounding boundary is unaffected.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@## main #328 +/- ##
=======================================
Coverage 99.55% 99.55% =======================================
Files 12 12 Lines 900 907 +7 =======================================
+ Hits 896 903 +7
Misses 4 4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Merging this PR will degrade performance by 14.55%
|
| Benchmark | BASE | HEAD | Efficiency | |
|---|---|---|---|---|
| ❌ | test_metric | 44.1 µs | 51.6 µs | -14.55% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing gaoflow:fix/metric-si-prefix-carry (4b69e61) with main (c2c410c)
Uh oh!
There was an error while loading. Please reload this page.
metric() to the next SI prefix when rounding reaches 1000Uh oh!
There was an error while loading. Please reload this page.
Summary
metric()can emit a four-digit mantissa with a too-small SI prefix:This contradicts the function's own docstring, which states the prefix is
1000 V/1000 kVare precisely the1230K-style outputs it promises to avoid, so the documented contract is the oracle here.Root cause
metric()selects the SI prefix bucket fromexponent = floor(log10(abs(value)))on the raw value, then rounds the mantissa to the requested precision. When the value sits just below a power-of-1000 boundary, the rounded mantissa becomes1000, but the prefix was already locked in from the pre-rounding exponent, and the code never re-checks whether rounding crossed into the next bucket.Fix
After computing the mantissa and its display precision, if
round(abs(mantissa), digits) >= 1000and a higher prefix is available (exponent < 30), advance to the next prefix bucket and re-divide. The top of the SI range (quetta,Q, 10^30) has no higher prefix, so theexponent < 30guard leaves it unchanged rather than indexing past the prefix table.Tests
Added four cases to the
test_metricparametrization (the carry cases above). They fail onmain('1000 V' != '1.00 kV', etc.) and pass with this change. The fulltests/test_number.pysuite passes (223 passed), so existingmetric()output, including the high-rangeQF/qAcases and the exact power-of-1000 inputs, is unchanged.Disclosure: I developed this fix with AI assistance, under my direction. I reviewed and verified the boundary behaviour (positive carry, the negative-exponent
mVpath, and the top-of-range quetta guard) and the regression run myself, and I am happy to adjust the approach.