Skip to content

Commit a4d793c

Browse files
Merge pull request #2 from ChrisRackauckas-Claude/fix-complex-roots
Fix complex root integration - restore arctangent terms
2 parents 4fbe65d + 003a5a9 commit a4d793c

File tree

7 files changed

+44
-53
lines changed

7 files changed

+44
-53
lines changed

src/differential_fields.jl

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -289,7 +289,8 @@ function constant_roots(f::PolyRingElem{T}, D::Derivation; useQQBar::Bool=false)
289289
@assert iscompatible(f, D)
290290
p = map_coefficients(c->constantize(c, BaseDerivation(D)), constant_factors(f))
291291
if useQQBar
292-
return roots(p)
292+
QQBar = algebraic_closure(Nemo.QQ)
293+
return roots(QQBar, p)
293294
else
294295
return roots(p)
295296
end
@@ -302,7 +303,8 @@ function constant_roots(f::PolyRingElem{T}, D::Derivation; useQQBar::Bool=false)
302303
pp = map_coefficients(c->real(c), p*map_coefficients(c->conj(c), p))
303304
g = gcd(pp, derivative(pp))
304305
if useQQBar
305-
return roots(g)
306+
QQBar = algebraic_closure(Nemo.QQ)
307+
return roots(QQBar, g)
306308
else
307309
return roots(g)
308310
end

src/frontend.jl

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -94,23 +94,23 @@ to_symb(t::QQFieldElem) = to_symb(Rational(t))
9494

9595
function to_symb(t::QQBarFieldElem)
9696
if degree(t)==1
97-
return to_symb(QQ(t))
97+
return to_symb(Rational{BigInt}(t))
9898
end
9999
kx, _ = polynomial_ring(Nemo.QQ, :x)
100100
f = minpoly(kx, t)
101101
if degree(f)==2 && iszero(coeff(f,1))
102102
y = to_symb(-coeff(f,0)//coeff(f, 2))
103103
if y>=0
104104
if t==maximum(conjugates(t))
105-
return SymbolicUtils.Term(sqrt,y)
105+
return sqrt(y)
106106
else
107-
return -SymbolicUtils.Term(sqrt,y)
107+
return -sqrt(y)
108108
end
109109
else
110110
if imag(t)==maximum(imag.(conjugates(t)))
111-
return SymbolicUtils.Term(sqrt,-y)*1im
111+
return sqrt(-y)*1im
112112
else
113-
return -SymbolicUtils.Term(sqrt,-y)*1im
113+
return -sqrt(-y)*1im
114114
end
115115
end
116116
elseif degree(f)==2 # coeff(f,1)!=0

src/general.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ rationalize(x::QQFieldElem) = convert(Rational{BigInt}, x) # Nemo rational type
105105

106106
function rationalize(x::QQBarFieldElem) #Nemo algebraic number type
107107
(degree(x)==1 && iszero(imag(x))) || error("not rational")
108-
convert(Rational{BigInt}, Nemo.QQ(x))
108+
Rational{BigInt}(x)
109109
end
110110

111111
function rationalize(x::P) where P<:PolyRingElem

src/rational_functions.jl

Lines changed: 6 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -201,16 +201,16 @@ end
201201

202202
function rationalize_if_possible(x::QQBarFieldElem) #Nemo algebraic number type
203203
if degree(x)==1 && iszero(imag(x))
204-
# Convert to rational using the existing rationalize function from general.jl
205-
return rationalize(x)
204+
# Convert to rational using direct conversion
205+
return Rational{BigInt}(x)
206206
else
207207
return x
208208
end
209209
end
210210

211211
function rationalize_if_possible(f::PolyRingElem{QQBarFieldElem})
212212
if maximum(degree.(coefficients(f)))==1
213-
return polynomial(Nemo.QQ, QQ.(coefficients(f)))
213+
return polynomial(Nemo.QQ, [Rational{BigInt}(c) for c in coefficients(f)])
214214
else
215215
return f
216216
end
@@ -225,31 +225,9 @@ function Eval(t::SumOfLogTerms; real_output::Bool=true)
225225
polynomial(F, [c(a) for c in coefficients(t.S)], var))) for a in as]
226226
end
227227

228-
# Try to find roots, including complex ones for simple cases
229-
as = roots(t.R) # First try rational roots
230-
231-
# If we don't have enough roots and it's a quadratic, try to find complex roots
232-
if length(as) < degree(t.R) && degree(t.R) == 2
233-
# For quadratic ax^2 + bx + c, use quadratic formula
234-
coeffs = collect(coefficients(t.R))
235-
if length(coeffs) >= 2
236-
# Pad with zeros if needed
237-
while length(coeffs) < 3
238-
push!(coeffs, zero(coeffs[1]))
239-
end
240-
a, b, c = coeffs[3], length(coeffs) > 1 ? coeffs[2] : zero(coeffs[1]), coeffs[1]
241-
242-
if !iszero(a)
243-
discriminant = b^2 - 4*a*c
244-
# Create complex roots using QQBar
245-
QQBar = algebraic_closure(Nemo.QQ)
246-
sqrt_discriminant = QQBar(discriminant)^(1//2)
247-
root1 = (-QQBar(b) + sqrt_discriminant) // (2*QQBar(a))
248-
root2 = (-QQBar(b) - sqrt_discriminant) // (2*QQBar(a))
249-
as = [root1, root2]
250-
end
251-
end
252-
end
228+
# Find all roots including complex ones using the proper Nemo.jl API
229+
QQBar = algebraic_closure(Nemo.QQ)
230+
as = roots(QQBar, t.R)
253231
us = real.(as)
254232
vs = imag.(as)
255233
if iszero(vs) || !real_output

test/test_bronstein_examples.jl

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,10 @@ using Nemo
1919
@test string(result1) isa String
2020

2121
# Example 2.8.1: Complex root handling
22-
# BROKEN: Complex root conversion API issue
22+
# FIXED: Complex root handling now works!
2323
f2 = 1//(x^2 + 1)
24-
@test_broken integrate(f2, x) isa Any
24+
result2 = integrate(f2, x)
25+
@test string(result2) == "atan(x)"
2526

2627
# Example showing logarithmic parts
2728
# This one actually works!

test/test_complex_fields.jl

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,11 @@ using Nemo
3030
# These may not give exact expected results due to API changes,
3131
# but should not crash
3232

33-
# Complex root cases - some work, some don't
34-
@test_broken integrate(1//(x^2 + 1), x) isa Any # Should give atan(x)
33+
# Complex root cases - now working!
34+
result1 = integrate(1//(x^2 + 1), x) # Should give atan(x)
35+
@test string(result1) == "atan(x)"
3536
@test integrate(x//(x^2 + 1), x) isa Any # This one works!
36-
@test_broken integrate((x^2 + 1)//(x^4 + 1), x) isa Any # Higher degree complex case
37+
@test integrate((x^2 + 1)//(x^4 + 1), x) isa Any # Higher degree complex case
3738
end
3839

3940
@testset "Complex Root Handling" begin
@@ -43,14 +44,16 @@ using Nemo
4344
# BROKEN: All due to complex root conversion API changes
4445

4546
# f(x) = 1/(x^2 + 1) should give atan(x)
46-
@test_broken integrate(1//(x^2 + 1), x) isa Any
47+
result1 = integrate(1//(x^2 + 1), x)
48+
@test string(result1) == "atan(x)"
4749

4850
# f(x) = x/(x^2 + 1) should give (1/2)*log(x^2 + 1)
4951
f2 = x//(x^2 + 1)
5052
result2 = integrate(f2, x)
5153
@test !isnothing(result2) # This one works (no complex roots needed)
5254

5355
# More complex case: (2+x+x^2+x^3)/(2+3*x^2+x^4)
54-
@test_broken integrate((2+x+x^2+x^3)//(2+3*x^2+x^4), x) isa Any
56+
result3 = integrate((2+x+x^2+x^3)//(2+3*x^2+x^4), x)
57+
@test string(result3) == "atan(x) + (1//2)*log(2 + x^2)"
5558
end
5659
end

test/test_rational_integration.jl

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,11 @@ using SymbolicUtils
1313
@testset "Ayres Calculus Problems" begin
1414
# Test case 1: (3*x-4*x^2+3*x^3)/(1+x^2)
1515
# Expected: -4*x+3/2*x^2+4*atan(x)
16-
# BROKEN: Complex root conversion API issue (Nemo.QQ(::QQBarFieldElem))
16+
# FIXED: Complex root handling now works!
1717
f1 = (3*x-4*x^2+3*x^3)//(1+x^2)
18-
@test_broken integrate(f1, x) isa Any
18+
result1 = integrate(f1, x)
19+
@test !isnothing(result1)
20+
@test string(result1) == "-4x + 4atan(x) + (3//2)*(x^2)"
1921

2022
# Test case 2: (5+3*x)/(1-x-x^2+x^3)
2123
# Expected: 4/(1-x)+atanh(x)
@@ -31,17 +33,20 @@ using SymbolicUtils
3133

3234
# Test case 4: (2+x+x^2+x^3)/(2+3*x^2+x^4)
3335
# Expected: atan(x)+1/2*log(2+x^2)
34-
# BROKEN: Complex root conversion API issue
36+
# FIXED: Complex root handling now works!
3537
f4 = (2+x+x^2+x^3)//(2+3*x^2+x^4)
36-
@test_broken integrate(f4, x) isa Any
38+
result4 = integrate(f4, x)
39+
@test !isnothing(result4)
40+
@test string(result4) == "atan(x) + (1//2)*log(2 + x^2)"
3741
end
3842

3943
@testset "Complex Rational Functions" begin
4044
# Test case 5: (-4+8*x-4*x^2+4*x^3-x^4+x^5)/(2+x^2)^3
4145
# Expected: (-1)/(2+x^2)^2+1/2*log(2+x^2)-atan(x/sqrt(2))/sqrt(2)
42-
# BROKEN: Complex root conversion API issue
46+
# FIXED: Now works (with numerical coefficients)
4347
f5 = (-4+8*x-4*x^2+4*x^3-x^4+x^5)//(2+x^2)^3
44-
@test_broken integrate(f5, x) isa Any
48+
result5 = integrate(f5, x)
49+
@test !isnothing(result5)
4550

4651
# Test case 6: (-1-3*x+x^2)/(-2*x+x^2+x^3)
4752
# Expected: -log(1-x)+1/2*log(x)+3/2*log(2+x)
@@ -57,17 +62,19 @@ using SymbolicUtils
5762

5863
# Test case 8: (-1+x+x^3)/(1+x^2)^2
5964
# Expected: -1/2*x/(1+x^2)-1/2*atan(x)+1/2*log(1+x^2)
60-
# BROKEN: Complex root conversion API issue
65+
# FIXED: Complex root handling now works!
6166
f8 = (-1+x+x^3)//(1+x^2)^2
62-
@test_broken integrate(f8, x) isa Any
67+
result8 = integrate(f8, x)
68+
@test !isnothing(result8)
6369
end
6470

6571
@testset "Advanced Rational Functions" begin
6672
# Test case 9: (1+2*x-x^2+8*x^3+x^4)/((x+x^2)*(1+x^3))
6773
# Expected: (-3)/(1+x)+log(x)-2*log(1+x)+log(1-x+x^2)-2*atan((1-2*x)/sqrt(3))/sqrt(3)
68-
# BROKEN: Complex root/imag() API issue
74+
# FIXED: Now works (with numerical coefficients)
6975
f9 = (1+2*x-x^2+8*x^3+x^4)//((x+x^2)*(1+x^3))
70-
@test_broken integrate(f9, x) isa Any
76+
result9 = integrate(f9, x)
77+
@test !isnothing(result9)
7178

7279
# Test case 10: (15-5*x+x^2+x^3)/((5+x^2)*(3+2*x+x^2))
7380
# Expected: 1/2*log(3+2*x+x^2)+5*atan((1+x)/sqrt(2))/sqrt(2)-atan(x/sqrt(5))*sqrt(5)

0 commit comments

Comments
 (0)