Incorrect doctest in sage/misc/functional.py

[orlitzky]

We currently have,

sage: a, b, c = var("a, b, c")
sage: v = vector([a, b, c])
sage: bool(norm(v).simplify_full() == sqrt(a^2 + b^2 + c^2))
True

which is true enough if a,b,c are real. The current doctest "works" because somewhere along the line, simplify_full() assumes that they're real. So in effect, this doctest is confirming incorrect behavior.

The proper fix is to explicitly assume that a,b,c are real.

---

Apply attachment: sage-trac_12845.patch and attachment: trac_12845-reviewer.patch.

CC: @kcrisman

Component: symbolics

Author: Michael Orlitzky

Reviewer: Karl-Dieter Crisman

Merged: sage-5.1.beta3

Issue created by migration from https://trac.sagemath.org/ticket/12845

[orlitzky]

Add assumptions and use simplify() instead of simplify_full().

[orlitzky]

comment:1

Attachment: sage-trac_12845.patch.gz

[orlitzky]

Author: Michael Orlitzky

[novoselt]

comment:2

Should there also be another ticket for fixing the bug of the real assumption?

[orlitzky]

comment:3

Replying to @novoselt:

Should there also be another ticket for fixing the bug of the real assumption?

Yes, there are at least two, #12737 and #12780. I separated this patch out because I wound up fixing the same doctest twice.

The first bug, "fixed" by #12737 is that simplify_radical() ignores the domain anyway and goes nuts. "Simplify" there is really a misnomer, so the fix that I settled on was to remove it from simplify_full():

sage: bool(norm(v).simplify_radical() == sqrt(a^2 + b^2 + c^2))
True

The second more sneaky bug, #12780, is that simplify_log() sets the Maxima domain to real before it performs its simplifications. Thus,

sage: bool(norm(v).simplify_log() == sqrt(a^2 + b^2 + c^2))    
True

I think the fix there is straightforward: don't do that. This was the only doctest affected by not setting the domain to real during simplify_log().

[kcrisman]

comment:5

Do you have to forget() anything for doctests to work properly? I think it's good form to do so at the end of any assumptions tests...

[kcrisman]

comment:6

Attachment: trac_12845-reviewer.patch.gz

Apply sage-trac_12845.patch and trac_12845-reviewer.patch.

[kcrisman]

Reviewer: Karl-Dieter Crisman

[orlitzky]

comment:7

Replying to @kcrisman:

Do you have to forget() anything for doctests to work properly? I think it's good form to do so at the end of any assumptions tests...

Yeah, thanks, I just forgot to add it.

[jdemeyer]

Merged: sage-5.1.beta3