Opened 11 years ago

Closed 10 years ago

# Incorrect doctest in sage/misc/functional.py

Reported by: Owned by: Michael Orlitzky Burcin Erocal major sage-5.1 symbolics Karl-Dieter Crisman sage-5.1.beta3 Michael Orlitzky Karl-Dieter Crisman N/A

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.

### comment:1 Changed 11 years ago by Michael Orlitzky

Authors: → Michael Orlitzky new → needs_review

### comment:2 follow-up:  3 Changed 11 years ago by Andrey Novoseltsev

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

### comment:3 in reply to:  2 Changed 11 years ago by Michael Orlitzky

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().

### comment:5 follow-up:  7 Changed 11 years ago by Karl-Dieter Crisman

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...

### comment:6 Changed 11 years ago by Karl-Dieter Crisman

Description: modified (diff) → Karl-Dieter Crisman needs_review → positive_review

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