#17556 closed enhancement (fixed)
Move simplify_log() from simplify_full() to simplify_real()
Reported by:  Michael Orlitzky  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 8 years ago by
Authors:  → Michael Orlitzky 

Branch:  → u/mjo/ticket/17556 
Commit:  → 044db1d2b7bc4e914e92ed7aa87629cbe958bc52 
Status:  new → needs_review 
comment:2 Changed 8 years ago by
Reviewers:  → Ralf Stephan 

Status:  needs_review → needs_work 
sage t long src/doc/de/thematische_anleitungen/sage_gymnasium.rst # 1 doctest failed
comment:3 Changed 8 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 8 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 Changed 8 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 8 years ago by
Branch:  u/mjo/ticket/17556 → u/rws/ticket/17556 

comment:7 Changed 8 years ago by
Commit:  044db1d2b7bc4e914e92ed7aa87629cbe958bc52 → 3616d00f582e175005d9b0182a1c1f4c124ebb56 

Status:  needs_work → 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:9 Changed 8 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 8 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 8 years ago by
Branch:  u/rws/ticket/17556 → 3616d00f582e175005d9b0182a1c1f4c124ebb56 

Resolution:  → fixed 
Status:  positive_review → closed 
comment:12 Changed 8 years ago by
Commit:  3616d00f582e175005d9b0182a1c1f4c124ebb56 

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