Opened 3 years ago

Closed 5 months ago

Last modified 5 months ago

#11934 closed defect (fixed)

Symbolic simplification error

Reported by: mjo Owned by: burcin
Priority: major Milestone: sage-5.13
Component: symbolics Keywords:
Cc: Merged in: sage-5.13.rc0
Authors: Michael Orlitzky Reviewers: Jeroen Demeyer
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by kcrisman)

I ran into this today with a real function. Sorry I don't have a shorter test case. The attached file should show a simplification which, as far as I can tell, is invalid.

sage: f = QQ(0.25)*(sqrt(2) - 2)*(x + 1)*x**3 - QQ(3)/QQ(8)*(sqrt(2) - 2)*(x + 1)*x**2 - 
QQ(0.25)*(sqrt(2) - 2)*(2*(3*sqrt(2) - 2)*x**2 - 2*(sqrt(2) - 1)*x + sqrt(-8*(4*sqrt(2) - 
7)*x**4 + 16*(3*sqrt(2) - 5)*x**3 - 8*(2*sqrt(2) - 3)*x - 4*x**2 + 4) - 4*sqrt(2) + 
2)*x**3/(sqrt(2)*x**2 + sqrt(2)*x - 2*sqrt(2)) - 1/24*(x + 1)**3*(x**3 - 3*x + 2) + 
QQ(3)/QQ(8)*(sqrt(2) - 2)*(2*(3*sqrt(2) - 2)*x**2 - 2*(sqrt(2) - 1)*x + sqrt(-8*(4*sqrt(2) 
- 7)*x**4 + 16*(3*sqrt(2) - 5)*x**3 - 8*(2*sqrt(2) - 3)*x - 4*x**2 + 4) - 4*sqrt(2) + 
2)*x**2/(sqrt(2)*x**2 + sqrt(2)*x - 2*sqrt(2)) - QQ(1)/QQ(16)*(x + 1)**2*(2*(sqrt(2) - 
3)*x**3 - (3*sqrt(2) - 8)*x**2 + 2*x + sqrt(2) - 4) + QQ(1)/QQ(8)*(x + 1)*sqrt(2) + 
QQ(1)/QQ(96)*(x**3 - 3*x + 2)*(2*(3*sqrt(2) - 2)*x**2 - 2*(sqrt(2) - 1)*x + sqrt(-8*
(4*sqrt(2) - 7)*x**4 + 16*(3*sqrt(2) - 5)*x**3 - 8*(2*sqrt(2) - 3)*x - 4*x**2 + 4) - 
4*sqrt(2) + 2)**3/(sqrt(2)*x**2 + sqrt(2)*x - 2*sqrt(2))**3 + 1/32*(2*(3*sqrt(2) - 
2)*x**2 - 2*(sqrt(2) - 1)*x + sqrt(-8*(4*sqrt(2) - 7)*x**4 + 16*(3*sqrt(2) - 5)*x**3 - 8*
(2*sqrt(2) - 3)*x - 4*x**2 + 4) - 4*sqrt(2) + 2)**2*(2*(sqrt(2) - 3)*x**3 - (3*sqrt(2) - 
8)*x**2 + 2*x + sqrt(2) - 4)/(sqrt(2)*x**2 + sqrt(2)*x - 2*sqrt(2))**2 - QQ(0.25)*x - 
QQ(1)/QQ(8)*(2*(3*sqrt(2) - 2)*x**2 - 2*(sqrt(2) - 1)*x + sqrt(-8*(4*sqrt(2) - 7)*x**4 + 
16*(3*sqrt(2) - 5)*x**3 - 8*(2*sqrt(2) - 3)*x - 4*x**2 + 4) - 4*sqrt(2) + 
2)*sqrt(2)/(sqrt(2)*x**2 + sqrt(2)*x - 2*sqrt(2)) + QQ(0.25)*(2*(3*sqrt(2) - 2)*x**2 - 2*
(sqrt(2) - 1)*x + sqrt(-8*(4*sqrt(2) - 7)*x**4 + 16*(3*sqrt(2) - 5)*x**3 - 8*(2*sqrt(2) - 
3)*x - 4*x**2 + 4) - 4*sqrt(2) + 2)/(sqrt(2)*x**2 + sqrt(2)*x - 2*sqrt(2)) - QQ(0.25)
sage: f.full_simplify()
-1/24*(sqrt(2)*x^8 - 2*(sqrt(2) - 3)*x^7 - (14*sqrt(2) - 15)*x^6 + 10*(9*sqrt(2) - 
13)*x^5 - (93*sqrt(2) - 128)*x^4 - 4*(9*sqrt(2) - 14)*x^3 + (58*sqrt(2) - 77)*x^2 + 4*
(sqrt(2) - 2)*x - sqrt(2*(4*sqrt(2) - 7)*x^2 + 4*(sqrt(2) - 2)*x - 1)*((16*I*sqrt(2) - 
28*I)*x^4 + (-24*I*sqrt(2) + 40*I)*x^3 + (8*I*sqrt(2) - 12*I)*x + 2*I*x^2 - 2*I) - 
8*sqrt(2) + 10)/(sqrt(2)*x^2 + 4*sqrt(2)*x + 4*sqrt(2))

Attachments (2)

simplify_error.sage (1.8 KB) - added by mjo 3 years ago.
A sage file that displays and attempts to plot the simplification
sage-trac_11934.patch (964 bytes) - added by mjo 8 months ago.
Doctest for a subexpression of the one in the ticket

Download all attachments as: .zip

Change History (13)

Changed 3 years ago by mjo

A sage file that displays and attempts to plot the simplification

comment:1 follow-up: Changed 3 years ago by kcrisman

  • Description modified (diff)

Can you be more specific about the invalidity? I think the plot errors are because of the imaginary pieces. Remember, these simplifications are not supposed to be 100 percent valid at all times; especially with roots there are branch issues, unfortunately. The f in question is pretty long - any sense as to where it might simplify in an unusual way?

comment:2 Changed 3 years ago by kcrisman

  • Description modified (diff)

comment:3 in reply to: ↑ 1 Changed 3 years ago by mjo

Replying to kcrisman:

Can you be more specific about the invalidity?

This seems to be the root of the problem. My function is real, the simplification is not:

sage: n(f(x=-0.5))
0.0175781250000000

sage: n(f.full_simplify()(x=-0.5))
0.0175781250000000 - 1.27567374911183e-18*I

I realize that the imaginary part is basically zero, but one of the simplifications has overstepped its bounds somewhere in that the expression is verifiably different pre- and post-simplification.

comment:4 Changed 2 years ago by jdemeyer

  • Milestone sage-4.7.3 deleted

Milestone sage-4.7.3 deleted

comment:5 Changed 2 years ago by mjo

  • Authors set to Michael Orlitzky
  • Dependencies set to #12322
  • Milestone set to sage-5.0
  • Status changed from new to needs_review

This really depends on #12737, but for the patch to apply nicely, #12322 should go in first.

comment:6 follow-up: Changed 11 months ago by kcrisman

  • Status changed from needs_review to needs_info

I'm questioning whether this really is fixing anything. First, it's still there with simplify_radical.

sage: f = sqrt(-8*(4*sqrt(2) - 7)*x^4 + 16*(3*sqrt(2) - 5)*x^3)
sage: f.subs(x=-1/2).n()
1.47861134210963
sage: f.simplify_radical().subs(x=-1/2).n()
1.47861134210963 - 9.05388323648765e-17*I

Secondly, the problem is really this. Notice I'm not simplifying anything at all here.

sage: h = I*x^(1/2)
sage: h(x=-1/2)
I*sqrt(-1/2)
sage: h(x=-1/2).n()
-0.707106781186548 + 4.32978028117747e-17*I

Needs work/info, but not because of your patch or #12737, but rather because this doesn't really treat the underlying issue. This could just be some floating point thing that is inherently impossible to avoid once one allows complex numbers (and since your original example is complex sometimes, the answers are going to be complex, unfortunately).

comment:7 in reply to: ↑ 6 Changed 10 months ago by mjo

Replying to kcrisman:

Needs work/info, but not because of your patch or #12737, but rather because this doesn't really treat the underlying issue. This could just be some floating point thing that is inherently impossible to avoid once one allows complex numbers (and since your original example is complex sometimes, the answers are going to be complex, unfortunately).

There aren't any floating point issues if you don't call n() on anything. The issue is still there with simplify_radical(), but that's because simplify_radical() is broken by design: it chooses a branch arbitrarily for the square root. This is what radcan() in Maxima is documented to do, but if you re-brand it as a simplification, it's a bug.

Plain simplify() works:

sage: f = sqrt(-8*(4*sqrt(2) - 7)*x^4 + 16*(3*sqrt(2) - 5)*x^3)
sage: f.simplify()
sqrt(-8*(4*sqrt(2) - 7)*x^4 + 16*(3*sqrt(2) - 5)*x^3)

As does simplify_trig():

sage: f.simplify_trig()
sqrt(-8*(4*sqrt(2) - 7)*x^4 + 16*(3*sqrt(2) - 5)*x^3)

And simplify_rational():

sage: f.simplify_rational()
sqrt(-8*(4*sqrt(2) - 7)*x^4 + 16*(3*sqrt(2) - 5)*x^3)

And simplify_log():

sage: f.simplify_log()
sqrt(-8*(4*sqrt(2) - 7)*x^4 + 16*(3*sqrt(2) - 5)*x^3)

It's only simplify_radical() that messes everything up (note I haven't called n() anywhere, so there are no numerical issues):

sage: f.simplify_radical()
2*I*sqrt((4*sqrt(2) - 7)*x - 6*sqrt(2) + 10)*sqrt(2)*x^(3/2)

I don't want a random branch of the square root when I ask for a simplification. I want a simplification, but only if possible! Without more information, you can't simplify that expression. The rest of the simplify functions don't do anything, but that's the correct thing to do here.

comment:8 Changed 8 months ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

Changed 8 months ago by mjo

Doctest for a subexpression of the one in the ticket

comment:9 Changed 8 months ago by mjo

  • Status changed from needs_info to needs_review

I just uploaded a slightly better doctest now that this is fixed.

comment:10 follow-up: Changed 5 months ago by jdemeyer

  • Dependencies #12322 deleted
  • Merged in set to sage-5.13.rc0
  • Resolution set to fixed
  • Reviewers set to Jeroen Demeyer
  • Status changed from needs_review to closed

This doesn't seem to depend on #12322 at all.

comment:11 in reply to: ↑ 10 Changed 5 months ago by mjo

Replying to jdemeyer:

This doesn't seem to depend on #12322 at all.

Both tickets add a doctest in exactly the same spot, so I added the dep to avoid fuzzing the patches. But you're right, the changes themselves are independent.

Note: See TracTickets for help on using tickets.