fix(solver): preserve exact π in periodic transcendental solve#1844
Open
s-celles wants to merge 1 commit into
Open
fix(solver): preserve exact π in periodic transcendental solve#1844s-celles wants to merge 1 commit into
s-celles wants to merge 1 commit into
Conversation
`symbolic_solve(cos(x) ~ 1//2, x)` returned
`1.0471975511965976 + 6.283185307179586·n` — a floating-point approximation —
instead of `π/3 + 2π·n`. The defect had three collaborating sites in the
periodic-equation path:
- `isolate` applied the left-inverse operator eagerly (`acos(1//2)` ⇒ Float64)
instead of keeping a symbolic term that `convert_consts` could canonicalise
downstream. Fixed by wrapping as `Symbolics.term(invop, sol; type = Real)`
on both the periodic and non-periodic branches.
- `fundamental_period` returned `2pi` / `1pi` — Julia promotes
`Int * Irrational{:π}` to `Float64`, so the period coefficient was
`6.2832…` / `3.1416…`. Fixed by returning `Symbolics.term(*, 2, pi)` and
`Symbolics.term(*, 1, pi)` for the `sin`/`cos`/`csc`/`sec` and `tan`/`cot`
families. The degree- and cycle-based variants (`sind`, `cosd`, `tand`,
`cotd`, `cospi`) are left unchanged — their periods are rationals.
- `_postprocess_root` rebuilt expression trees via
`SymbolicUtils.maketerm`, which folds known-function calls on constant
operands — so `maketerm(BasicSymbolic, /, [π, 3], nothing)` collapsed back
to `1.0471…` on the second iteration of the `postprocess_root` fixed-point
loop. Fixed by rebuilding via `Symbolics.term` when any argument is an
irrational math constant (`π` or `e`).
- The canonical-value lookup table in `postprocess.jl` was missing `3π/4`
(`acos(-√2/2)`); added.
- The complex-roots angular factor in `isolate` for `x^n ~ y` used plain
`pi` which promoted to Float64; wrapped symbolically.
Tests: new `@testset "Exact transcendental solve (JuliaSymbolics#1842)"` in
`test/solver.jl` covers every canonical `sin`/`cos`/`tan` right-hand side,
the symbolic period coefficient, and non-canonical preservation (152
assertions). The pre-existing `@test_broken` at `test/solver.jl:632`
(composed-argument `sin+cos` case) still fails after this fix — it exercises
the `attract` path which is out of scope here; a comment now documents that.
Docs: new example in `docs/src/manual/solver.md`.
Closes JuliaSymbolics#1842.
Author
(@v1.12) pkg> activate .
Activating project at `~/work/tries/Symbolics.jl`
julia> using Symbolics
julia> @variables x
1-element Vector{Num}:
x
julia> Symbolics.symbolic_solve(cos(x) ~ 1//2, x)
[ Info: var"##283" ϵ Ζ
1-element Vector{SymbolicUtils.BasicSymbolicImpl.var"typeof(BasicSymbolicImpl)"{SymReal}}:
π / 3 + var"##283"*(2*π) |
Author
|
Be aware that |
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.
symbolic_solve(cos(x) ~ 1//2, x)returned1.0471975511965976 + 6.283185307179586·n— a floating-point approximation — instead ofπ/3 + 2π·n. The defect had three collaborating sites in the periodic-equation path:isolateapplied the left-inverse operator eagerly (acos(1//2)⇒ Float64) instead of keeping a symbolic term thatconvert_constscould canonicalise downstream. Fixed by wrapping asSymbolics.term(invop, sol; type = Real)on both the periodic and non-periodic branches.fundamental_periodreturned2pi/1pi— Julia promotesInt * Irrational{:π}toFloat64, so the period coefficient was6.2832…/3.1416…. Fixed by returningSymbolics.term(*, 2, pi)andSymbolics.term(*, 1, pi)for thesin/cos/csc/secandtan/cotfamilies. The degree- and cycle-based variants (sind,cosd,tand,cotd,cospi) are left unchanged — their periods are rationals._postprocess_rootrebuilt expression trees viaSymbolicUtils.maketerm, which folds known-function calls on constant operands — somaketerm(BasicSymbolic, /, [π, 3], nothing)collapsed back to1.0471…on the second iteration of thepostprocess_rootfixed-point loop. Fixed by rebuilding viaSymbolics.termwhen any argument is an irrational math constant (πore).The canonical-value lookup table in
postprocess.jlwas missing3π/4(acos(-√2/2)); added.The complex-roots angular factor in
isolateforx^n ~ yused plainpiwhich promoted to Float64; wrapped symbolically.Tests: new
@testset "Exact transcendental solve (#1842)"intest/solver.jlcovers every canonicalsin/cos/tanright-hand side, the symbolic period coefficient, and non-canonical preservation (152 assertions). The pre-existing@test_brokenattest/solver.jl:632(composed-argumentsin+coscase) still fails after this fix — it exercises theattractpath which is out of scope here; a comment now documents that.Docs: new example in
docs/src/manual/solver.md.Helps #1842.