Add approx_rational for rational approximations of Computables#11
Add approx_rational for rational approximations of Computables#11TimTheBig wants to merge 4 commits into
approx_rational for rational approximations of Computables#11Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces the approx_rational method to Computable in src/computable/node.rs, which provides a dyadic rational approximation within a specified precision. It includes a fast-path optimization for exact-rational leaves with power-of-two denominators and adds comprehensive unit tests. The feedback suggests simplifying the fast-path check by reusing the existing dyadic_denominator_shift method on Rational to improve code maintainability.
|
What is the goal here? Real::exact_rational_or_approx* ignores Real.rational for non-One classes. In src/real/arithmetic.rs:1352 and src/real/arithmetic.rs:1385, the code approximates only the stored/computed class payload. But Real is rational * class, and many symbolic constants are scaled this way, for example Real::tau() is 2 * Pi with a Computable::pi() payload. The PR returns 201/64 for Real::tau().exact_rational_or_approx(-6), but the correct result is 201/32. Same bug affects borrowed form, pi/2, scaled logs/sqrts, and generic symbolic scaled values. Real.fold() already manages approximation at the level of Real, which is what you need to correctly address the above issue. |
The goal is to provide both convenience and correctness. I could see others making this function so why not make an internal one that takes advantage of shortcuts. As for why you would need it, sometimes you need a rational-only operation that an |
No description provided.