Opened 2 years ago
Last modified 4 months ago
#27897 needs_work defect
sqrt(abs(1+I)^2 + 14) evaluates to sqrt(16) instead of 4
Reported by:  rburing  Owned by:  

Priority:  major  Milestone:  
Component:  symbolics  Keywords:  sqrt, abs, complex, hold, holding 
Cc:  Merged in:  
Authors:  Dave Morris  Reviewers:  
Report Upstream:  N/A  Work issues:  
Branch:  public/27897 (Commits, GitHub, GitLab)  Commit:  a3303105570ca4ad4fecb896cb19543029c3da00 
Dependencies:  Stopgaps: 
Description
There is some weird undesirable "holding" going on:
sage: sqrt(abs(1+I)^2 + 14) sqrt(16)
Found in Ask SageMath question #46698: Sagemath not evaluating complicated expression.
Change History (7)
comment:1 Changed 2 years ago by
comment:2 Changed 2 years ago by
 Milestone sage8.8 deleted
As the Sage8.8 release milestone is pending, we should delete the sage8.8 milestone for tickets that are not actively being worked on or that still require significant work to move forward. If you feel that this ticket should be included in the next Sage release at the soonest please set its milestone to the next release milestone (sage8.9).
comment:3 Changed 4 months ago by
 Branch set to public/27897
comment:4 Changed 4 months ago by
 Commit set to 357ba2ff7d46ac72cd9ff5968b4755f39554fabf
 Status changed from new to needs_review
I think the problem is gone:
sage: sqrt(abs(1+I)^2 + 14) 4
This is with 9.3b6 on MacOS 10.15.7, but I get sqrt(16)
with 9.2 on CoCalc
, so something seems to have changed since then.
The PR adds a doctest. If the patchbots turn green, then I think we can close this ticket.
New commits:
357ba2f  doctest trac 27897 sqrt(16)

comment:5 Changed 4 months ago by
I don't have 9.3beta6 here, but I think the definition/parent of I
has changed, which may well have hidden this bug instead of fixing it. Could you please check sqrt(abs(1+SR(I))^2 + 14)
? The doctest would have to take this form (or similar) as well.
comment:6 Changed 4 months ago by
 Commit changed from 357ba2ff7d46ac72cd9ff5968b4755f39554fabf to a3303105570ca4ad4fecb896cb19543029c3da00
Branch pushed to git repo; I updated commit sha1. New commits:
a330310  review correction

comment:7 Changed 4 months ago by
 Status changed from needs_review to needs_work
You're right  thanks for catching this! With 9.3b6, we still have:
sage: sqrt(abs(1 + SR.I())^2 + 14) sqrt(16)
Ticket #18036 changed the parent of I to be a number field.
It looks like there is some double wrapping happening here:
here things seem to be still reasonable. However, this last result is "double wrapped":
and it looks like that's too deep for
sqrt
to see through. It doesn't happen for this case:that could be a red herring, though:
It means that
both lead to unexpectedly "held" expressions, but with different structures:
where
so perhaps both problems are solved if
sqrt
on anSR
element would look a little deeper.