#17556 closed enhancement (fixed)
Move simplify_log() from simplify_full() to simplify_real()
Reported by:  mjo  Owned by:  

Priority:  major  Milestone:  sage6.5 
Component:  symbolics  Keywords:  
Cc:  Merged in:  
Authors:  Michael Orlitzky  Reviewers:  Ralf Stephan 
Report Upstream:  N/A  Work issues:  
Branch:  3616d00 (Commits, GitHub, GitLab)  Commit:  
Dependencies:  Stopgaps: 
Description
The simplify_log
function assumes that its argument is real; otherwise the log contraction operation is invalid. For example,
sage: x,y = SR.var('x,y') sage: assume(y, 'complex') sage: f = log(x*y)  (log(x) + log(y)) sage: f(x=1, y=i) 2*I*pi sage: f.simplify_log() 0
Now that we have a simplify_real()
method, why not move simplify_log()
there instead?
In the process, the newer simplify_rectform()
can be added to simplify_full()
.
I'm working on a patch for this.
Change History (12)
comment:1 Changed 6 years ago by
 Branch set to u/mjo/ticket/17556
 Commit set to 044db1d2b7bc4e914e92ed7aa87629cbe958bc52
 Status changed from new to needs_review
comment:2 Changed 6 years ago by
 Reviewers set to Ralf Stephan
 Status changed from needs_review to needs_work
sage t long src/doc/de/thematische_anleitungen/sage_gymnasium.rst # 1 doctest failed
comment:3 Changed 6 years ago by
I'm going to need some help from a native speaker to fix that. The context of that sentence is discussing the simplify_full()
method:
Falls wir alle Terme so weit wie möglich vereinfachen möchten, erreichen wir dies mit der ``simplify_full()`` Funktion:: sage: (sin(x)^2 + cos(x)^2).simplify_full() 1
Then the failing test mentions (I think) that simplify_full()
will also do something with the logarithms:
Dabei werden auch Additionstheoreme für trigonometrische Funktionen und manche Logarithmengesetze eingesetzt:: sage: var('x, y, z') (x, y, z) sage: (sin(x + y)/(log(x) + log(y))).simplify_full() (cos(y)*sin(x) + cos(x)*sin(y))/log(x*y) sage: (sin(x)^2 + cos(x)^2).simplify_full() 1
but of course that's not the case anymore because the log contraction isn't necessarily valid if x
or y
is complex (what's the z
for???).
One possible solution would be to mention simplify_real()
here. Note that the sin^2 + cos^2 == 1
identity is mentioned and tested twice in a row. Maybe we could drop the second one, mention something about simplify_real()
being used when the variables are real, and then show an example of,
sage: (log(x) + log(y)).simplify_real() log(x*y)
Another possible solution is to get rid of the log()
s and just don't mention them. That takes slightly less creativity but does lose a useful example =)
comment:4 followups: ↓ 5 ↓ 7 Changed 6 years ago by
Probably something like this would be appropriate (Ralf, I assume your German is "more native" than mine and can confirm):
Mit der verwandten Funktion ``simplify_real()`` werden auch Additionstheoreme ...
and then use simplify_real()
there.
That said, I'm not 100% convinced that simplify_full()
should be "only the best" simplifications. One of the distinct advantages of that function is that it did pretty much everything you wanted. If we keep on changing what you need to do for various simplifications, it will become quite annoying to use Sage for such things in the classroom. (Not to mention making various books immediately invalid.)
comment:5 in reply to: ↑ 4 Changed 6 years ago by
Replying to kcrisman:
That said, I'm not 100% convinced that
simplify_full()
should be "only the best" simplifications. One of the distinct advantages of that function is that it did pretty much everything you wanted. If we keep on changing what you need to do for various simplifications, it will become quite annoying to use Sage for such things in the classroom. (Not to mention making various books immediately invalid.)
I don't like removing them, but I do think we should try to avoid answers that are clearly wrong under the "simplify" name, like the example in the ticket description. We just don't have a lot of good simplifications available that work for complex variables. I did add simplify_rectform()
to simplify_full()
in this ticket as compensation.
In any case, this is a much smaller change than it might seem. I mention in the commit messages that the 'one' algorithm was passed to simplify_log()
via simplify_full()
, which means that log(x) + log(y)
would be contracted to log(x*y)
but 2*log(x) + log(y)
wouldn't. Why that choice was made we'll never know, but the result is that the bug was pretty rare. A useful simplification was also exceedingly rare.
I think the change only affected two doctests (including the German one), and in those cases it's not clear that the variables were supposed to be real, so the answers may be wrong anyway =)
With simplify_log()
moved into simplify_real()
, I've made it use the default algorithm. So now 2*log(x) + log(y)
will be simplified to log(x^2*y)
as well. That makes this particular simplification much more useful to people who use it. Before you would have had to dig into the simplify_full()
and simplify_log()
source to figure out a) why you weren't getting it, and, b) how to make it happen (with a separate call to simplify_log()
).
So tl;dr I felt bad removing two things from simplify_full()
in a row but this one nobody will notice. And I added one back!
comment:6 Changed 6 years ago by
 Branch changed from u/mjo/ticket/17556 to u/rws/ticket/17556
comment:7 in reply to: ↑ 4 Changed 6 years ago by
 Commit changed from 044db1d2b7bc4e914e92ed7aa87629cbe958bc52 to 3616d00f582e175005d9b0182a1c1f4c124ebb56
 Status changed from needs_work to positive_review
Oops, should have used a public branch.
Replying to kcrisman:
Probably something like this would be appropriate (Ralf, I assume your German is "more native" than mine and can confirm):
Mit der verwandten Funktion ``simplify_real()`` werden auch Additionstheoreme ...and then use
simplify_real()
there.
This patch adds such a test, and the sentence reads translated: With the related function simplify_real() also logarithmic addition theorems are applied, which hold for real values.
Since it's practically what you both proposed I'm including it with my positive review.
New commits:
3616d00  17556: reviewer's patch: adapt text and test to changed behaviour

comment:8 Changed 6 years ago by
The "die nur mit reellen Werten erlaubt sind" is perfect, great!
comment:9 Changed 6 years ago by
Sorry to be so late. I think the following does not sound optimal:
Mit der verwandten Funktion ``simplify_real()`` werden auch Additionstheoreme bei Logarithmen angewandt, die nur mit reellen Werten erlaubt sind::
I think in "Additionstheoreme bei Logarithmen", the "bei" is wrong, and also I wouldn't say that a theorem is "erlaubt".
What do you think about "werden auch Additionstheoreme verwendet, die nur für reellwertige Logarithmen gelten"? Or at least "werden auch solche Additionstheoreme für Logarithmen angewandt, die nur mit reellen Werten gelten"? (or "anwendbar sind" instead of "gelten")
comment:10 Changed 6 years ago by
Do I understand correctly that the purpose of this ticket is not to fix further errors in sage_gymnasium
? I just noticed that the example in the section on "Partialbruchzerlegung" gives a wrong result.
comment:11 Changed 6 years ago by
 Branch changed from u/rws/ticket/17556 to 3616d00f582e175005d9b0182a1c1f4c124ebb56
 Resolution set to fixed
 Status changed from positive_review to closed
comment:12 Changed 6 years ago by
 Commit 3616d00f582e175005d9b0182a1c1f4c124ebb56 deleted
How about Additionstheoreme, deren Benutzung nur mit reellen Werten erlaubt sind
or gelten
? But I didn't learn my math in German  deren Hypothesen nur für reelen Werte ...
? Anyway, Simon, definitely feel free to open another ticket to make this better as well as fixing anything else in sage_gymnasium
.
Not too much had to be changed for this. Since the 'one' algorithm was used for
simplify_log()
withinsimplify_full()
, it rarely had any effect.New commits:
Trac #17556: Remove simplify_log() from simplify_full().
Trac #17556: Add simplify_rectform() to simplify_full().
Trac #17556: Add Expression.simplify_log() to Expression.simplify_real().