Move simplify_log() from simplify_full() to simplify_real()

[orlitzky]

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.

Component: symbolics

Author: Michael Orlitzky

Branch: 3616d00

Reviewer: Ralf Stephan

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

[orlitzky]

Branch: u/mjo/ticket/17556

[orlitzky]

Author: Michael Orlitzky

[orlitzky]

comment:1

Not too much had to be changed for this. Since the 'one' algorithm was used for simplify_log() within simplify_full(), it rarely had any effect.

---

New commits:

fbc0a40	Trac #17556: Remove simplify_log() from simplify_full().
3ea1ded	Trac #17556: Add simplify_rectform() to simplify_full().
044db1d	Trac #17556: Add Expression.simplify_log() to Expression.simplify_real().

[orlitzky]

Commit: 044db1d

[rwst]

Reviewer: Ralf Stephan

[rwst]

comment:2

sage -t --long src/doc/de/thematische_anleitungen/sage_gymnasium.rst  # 1 doctest failed

[orlitzky]

comment:3

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 =)

[kcrisman]

comment:4

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

[orlitzky]

comment:5

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!

[rwst]

Changed branch from u/mjo/ticket/17556 to u/rws/ticket/17556

[rwst]

comment:7

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

[rwst]

Changed commit from 044db1d to 3616d00

[kcrisman]

comment:8

The "die nur mit reellen Werten erlaubt sind" is perfect, great!

[simon-king-jena]

comment:9

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")

[simon-king-jena]

comment:10

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.

[vbraun]

Changed branch from u/rws/ticket/17556 to 3616d00

[kcrisman]

Changed commit from 3616d00 to none

[kcrisman]

comment:12

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.