-
Notifications
You must be signed in to change notification settings - Fork 2
[cli] Use working precision for intermediate calculation #182
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,6 +21,7 @@ Evaluates a Reverse Polish Notation token list using BigDecimal arithmetic. | |
| """ | ||
|
|
||
| from decimo import BDec | ||
| from decimo.rounding_mode import RoundingMode | ||
|
|
||
| from .tokenizer import ( | ||
| Token, | ||
|
|
@@ -170,13 +171,17 @@ fn _call_func( | |
| fn evaluate_rpn(rpn: List[Token], precision: Int) raises -> BDec: | ||
| """Evaluate an RPN token list using BigDecimal arithmetic. | ||
|
|
||
| All numbers are BigDecimal. Division uses `true_divide` with | ||
| the caller-supplied precision. | ||
| Internally uses `working_precision = precision + GUARD_DIGITS` for all | ||
| computations to absorb intermediate rounding errors. The caller is | ||
| responsible for rounding the final result to `precision` significant | ||
| digits (see `final_round`). | ||
|
|
||
| Raises: | ||
| Error: On division by zero, missing operands, or other runtime | ||
| errors — with source position when available. | ||
| """ | ||
| comptime GUARD_DIGITS = 9 # Word size | ||
| var working_precision = precision + GUARD_DIGITS # working precision | ||
| var stack = List[BDec]() | ||
|
|
||
| for i in range(len(rpn)): | ||
|
|
@@ -187,9 +192,9 @@ fn evaluate_rpn(rpn: List[Token], precision: Int) raises -> BDec: | |
|
|
||
| elif kind == TOKEN_CONST: | ||
| if rpn[i].value == "pi": | ||
| stack.append(BDec.pi(precision)) | ||
| stack.append(BDec.pi(working_precision)) | ||
| elif rpn[i].value == "e": | ||
| stack.append(BDec.e(precision)) | ||
| stack.append(BDec.e(working_precision)) | ||
| else: | ||
| raise Error( | ||
| "Error at position " | ||
|
|
@@ -240,7 +245,13 @@ fn evaluate_rpn(rpn: List[Token], precision: Int) raises -> BDec: | |
| ) | ||
| var b = stack.pop() | ||
| var a = stack.pop() | ||
| stack.append(a * b) | ||
| var product = a * b | ||
| # Multiplication can grow digits unboundedly; trim to | ||
| # working precision to prevent intermediate blowup. | ||
| product.round_to_precision( | ||
| working_precision, RoundingMode.half_even(), False, False | ||
| ) | ||
| stack.append(product^) | ||
|
|
||
| elif kind == TOKEN_SLASH: | ||
| if len(stack) < 2: | ||
|
|
@@ -257,7 +268,7 @@ fn evaluate_rpn(rpn: List[Token], precision: Int) raises -> BDec: | |
| + ": division by zero" | ||
| ) | ||
| var a = stack.pop() | ||
| stack.append(a.true_divide(b, precision)) | ||
| stack.append(a.true_divide(b, working_precision)) | ||
|
|
||
| elif kind == TOKEN_CARET: | ||
| if len(stack) < 2: | ||
|
|
@@ -268,10 +279,10 @@ fn evaluate_rpn(rpn: List[Token], precision: Int) raises -> BDec: | |
| ) | ||
| var b = stack.pop() | ||
| var a = stack.pop() | ||
| stack.append(a.power(b, precision)) | ||
| stack.append(a.power(b, working_precision)) | ||
|
|
||
| elif kind == TOKEN_FUNC: | ||
| _call_func(rpn[i].value, stack, precision, rpn[i].position) | ||
| _call_func(rpn[i].value, stack, working_precision, rpn[i].position) | ||
|
|
||
| else: | ||
| raise Error( | ||
|
|
@@ -290,19 +301,45 @@ fn evaluate_rpn(rpn: List[Token], precision: Int) raises -> BDec: | |
| return stack.pop() | ||
|
|
||
|
|
||
| fn evaluate(expr: String, precision: Int = 50) raises -> BDec: | ||
| fn final_round( | ||
| value: BDec, | ||
| precision: Int, | ||
| rounding_mode: RoundingMode = RoundingMode.half_even(), | ||
| ) raises -> BDec: | ||
| """Round a BigDecimal to `precision` significant digits. | ||
|
|
||
| This should be called on the result of `evaluate_rpn` before | ||
| displaying it to the user, so that guard digits are removed and | ||
| the last visible digit is correctly rounded. | ||
| """ | ||
| if value.is_zero(): | ||
| return value.copy() | ||
| var result = value.copy() | ||
| result.round_to_precision(precision, rounding_mode, False, False) | ||
| return result^ | ||
|
Comment on lines
+304
to
+319
|
||
|
|
||
|
|
||
| fn evaluate( | ||
| expr: String, | ||
| precision: Int = 50, | ||
| rounding_mode: RoundingMode = RoundingMode.half_even(), | ||
| ) raises -> BDec: | ||
| """Evaluate a math expression string and return a BigDecimal result. | ||
|
|
||
| This is the main entry point for the calculator engine. | ||
| It tokenizes, parses (shunting-yard), and evaluates (RPN) the expression. | ||
| The result is rounded to `precision` significant digits. | ||
|
|
||
| Args: | ||
| expr: The math expression to evaluate (e.g. "100 * 12 - 23/17"). | ||
| precision: The number of decimal digits for division (default: 50). | ||
| precision: The number of significant digits (default: 50). | ||
| rounding_mode: The rounding mode for the final result | ||
| (default: half_even). | ||
|
|
||
| Returns: | ||
| The result as a BigDecimal. | ||
| The result as a BigDecimal, rounded to `precision` significant digits. | ||
| """ | ||
| var tokens = tokenize(expr) | ||
| var rpn = parse_to_rpn(tokens^) | ||
| return evaluate_rpn(rpn^, precision) | ||
| var result = evaluate_rpn(rpn^, precision) | ||
| return final_round(result, precision, rounding_mode) | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -12,9 +12,10 @@ | |||||
| from sys import exit | ||||||
|
|
||||||
| from argmojo import Arg, Command | ||||||
| from decimo.rounding_mode import RoundingMode | ||||||
| from calculator.tokenizer import tokenize | ||||||
| from calculator.parser import parse_to_rpn | ||||||
| from calculator.evaluator import evaluate_rpn | ||||||
| from calculator.evaluator import evaluate_rpn, final_round | ||||||
| from calculator.display import print_error | ||||||
|
|
||||||
|
|
||||||
|
|
@@ -58,7 +59,7 @@ fn _run() raises: | |||||
|
|
||||||
| # Named option: decimal precision | ||||||
|
||||||
| # Named option: decimal precision | |
| # Named option: number of significant digits |
Copilot
AI
Mar 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CLI now defines --precision as “number of significant digits”, but the --pad path still pads the fractional part to precision decimal places (via _pad_to_precision(..., precision)). This makes --pad inconsistent with the new precision semantics and can produce more than precision significant digits. Consider either implementing padding in significant-digits terms (e.g., using round_to_precision(..., fill_zeros_to_precision=True)), or introducing a separate --decimal-places/--scale flag for the current padding behavior.
| Arg("precision", help="Number of significant digits (default: 50)") | |
| Arg("precision", help="Number of decimal places (default: 50)") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GUARD_DIGITS = 9is a key accuracy/performance tradeoff but the comment “Word size” is unclear in this context (guard digits aren’t a word size unless you mean one base-1e9 BigUInt word ≈ 9 decimal digits). Please clarify the rationale (and ideally link it to the BigUInt base/word representation) so future changes don’t accidentally break precision guarantees.