Opened 11 years ago

Closed 8 years ago

exp(I*pi*n).simplify_exp() doesn't simplify

Reported by: Owned by: ddrake burcin major sage-6.2 symbolics maxima symbolics exponentials sd40.5 kcrisman, mjo Michael Orlitzky Ralf Stephan N/A 2fb52ed 2fb52ed22e39c63908bb612bb91ca9fd4126f477 #13061

Description

At https://groups.google.com/d/topic/sage-support/Pkn1o1kAcyE/discussion I noted that currently (4.7.1 and 4.7.2.alpha2), we have this:

```sage: n = var('n')
sage: assume(n, 'integer')
sage: exp(I*n*pi).simplify_exp()
e^(I*pi*n)
```

I was really hoping for `cos(pi*n)`, or `(-1)^n`.

If you are serious about asking Maxima, it will simplify it, though:

```sage: A = exp(I*pi*n)
sage: A.maxima_methods().rectform()
(-1)^n
```

We should somehow wrap rectform or make it easier to access. At a minimum, we should add something to the documentation to mention this possibility.

comment:2 Changed 10 years ago by mjo

• Status changed from new to needs_review

This is sort of a work-in-progress, but I'd appreciate comments on the approach.

comment:3 Changed 10 years ago by kcrisman

Hmm. Maybe for a first try we should just provide `simplify_rectform` or something more felicitously named regardless for now, and then discuss elsewhere whether it's useful to add this parameter. I'm not sure that string length is a good measure of complexity, though I'm not sure I have a better idea (other than total number of vertices of the expression tree, I suppose). In Maxima I don't think they do this, they just assume the user is smart enough to tell if it's simpler. As long as this isn't in the default simplification (not to reopen the `simplify_full` issue!), we might not want to open up a whole new file of complexity of a symbolic expression without input from experts in such things.

What do you think? I do like having this available, Dan isn't the first to ask. There are also some custom methods on some other ticket for pretty comprehensive dealing with the trig-complex issue, I can't remember where it is and don't have time to find it, though. You might want to do some snooping on sage-devel archives or on Trac for these.

comment:4 Changed 10 years ago by mjo

I tried it without the complexity measure first, but this function has a tendency to make expressions more complicated. If we're ever going to use it in e.g. `full_simplify()`, we need to be a little more selective about which results we accept.

It turns out, string length is actually a pretty good measure.

I implemented something close to the number of vertices in the expression tree:

```from types import ListType

def flatten_list(l):
result = []

for item in l:
if isinstance(item, ListType):
result += flatten_list(item)
else:
result += [item]

return result

def flatten_operands(operands):
return operands + [flatten_operands(g.operands()) for g in operands]

def all_operands(expr):
return flatten_list(flatten_operands(expr.operands()))
```

and then tried the `len(all_operands(expr))` complexity measure. It also works, but is much much much slower and doesn't seem "better" than string length really. We could add that to the `complexity_measures` module as an option, but the dumb solution seems to work too.

(The way that I'm testing, by the way, is to replace `simplify()` with `simplify_rect()` and watch the failing doctests. Many of them are promising.)

Admittedly adding a new module just for this feels like overkill, but the alternatives weren't too appealing:

• Default to `lambda x: len(str(x))`
• Default to `None`, and use the string length in that case. Requires the user to pass `lambda x: 1` to force the simplification (we'd also have to use less-than-or-equal-to in the function body).
• Remove the measure altogether; this usually wasn't a simplification.

Down the line, `simplify_full()` could potentially try `simplify_foo()` with every complexity measure, and return the expression that's simplest according to the measure you gave `simplify_full()`.

comment:5 Changed 10 years ago by kcrisman

Interesting, and good longer-range ideas.

Apropos slowness, that might be worth testing anyway for this. `full_simplify` is probably really slow, but testing two different strings like this could make it really slow (not that that is done here). The "down the line" would probably be super-slow for that reason.

Also, along the lines of plotting and William's recent email complaining about how 2D plotting got away from not caring about the backend, it might be worth thinking about how to allow a number of backends for simplification as well... just a thought.

comment:6 Changed 10 years ago by davidloeffler

Failing doctest (looks like a typo, should be complexity_measures):

```File "/home/patchbot/sage-5.0.beta3/devel/sage-11785/sage/symbolic/complexity_measures.py", line 30:
sage: from sage.symbolic.complexity_measure import string_length
Exception raised:
Traceback (most recent call last):
File "/home/patchbot/sage-5.0.beta3/local/bin/ncadoctest.py", line 1231, in run_one_test
self.run_one_example(test, example, filename, compileflags)
File "/home/patchbot/sage-5.0.beta3/local/bin/sagedoctest.py", line 38, in run_one_example
OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags)
File "/home/patchbot/sage-5.0.beta3/local/bin/ncadoctest.py", line 1172, in run_one_example
compileflags, 1) in test.globs
File "<doctest __main__.example_1[2]>", line 1, in <module>
from sage.symbolic.complexity_measure import string_length###line 30:
sage: from sage.symbolic.complexity_measure import string_length
ImportError: No module named complexity_measure
**********************************************************************
```

comment:7 Changed 10 years ago by davidloeffler

• Status changed from needs_review to needs_work

comment:8 Changed 10 years ago by mjo

• Status changed from needs_work to needs_review

Whoops, that was a last-minute change to match the naming of groups,rings, etc. I fixed the 's' and renamed the method to `simplify_rectform()` per comment 3.

comment:9 follow-up: ↓ 10 Changed 10 years ago by ddrake

• Status changed from needs_review to needs_work

I will ask some other people about the complexity measure stuff, but I think your default is reversed: for anyone who knows a little Maxima, they will expect something like `simplify_rectform()` to actually return what Maxima's `rectform` returns -- but the default isn't to do that.

Here's my proposal: by default, call Maxima's `rectform` and return that. Don't import anything from `complexity_measures.py` by default, but include documentation showing the user how to import that and use it in the keyword argument.

(Also, how hard is it to count the number of operands in an expression? It's just `len`, right? If it isn't too hard, perhaps the documentation could show how to use that as a complexity measure.)

comment:10 in reply to: ↑ 9 Changed 10 years ago by mjo

I will ask some other people about the complexity measure stuff, but I think your default is reversed: for anyone who knows a little Maxima, they will expect something like `simplify_rectform()` to actually return what Maxima's `rectform` returns -- but the default isn't to do that.

Here's my proposal: by default, call Maxima's `rectform` and return that. Don't import anything from `complexity_measures.py` by default, but include documentation showing the user how to import that and use it in the keyword argument.

But why introduce a default that's not useful? Unless you're setting up a toy example to show what `rectform` does, most expressions will be made uglier. Maxima's `rectform()` isn't labeled a simplification; if we're going to call it one, I think it should simplify.

If we call it a simplification and have it only do `rectform()` by default, we're essentially requiring the user to both read the docs and pass `make_it_work=True` on every invocation.

How many of our users are there that,

1. Know maxima and are familiar with its `rectform()` and don't know about expr.maxima_methods().rectform() and would expect `simplify_rectform()` to just call `rectform()`?
2. Use `simplify_foo()` and just want their expressions to get simpler.

I suspect (b) dwarfs (a).

I wouldn't mind if we just called it `Expression.rectform()` and documented what it does, but I also don't think that it would be as useful.

(Also, how hard is it to count the number of operands in an expression? It's just `len`, right? If it isn't too hard, perhaps the documentation could show how to use that as a complexity measure.)

It depends on how thorough you want to be. A simple `len(expr)` will return the number of top-level operands. If you want to count all operands at any level, you either have to recurse (as I did), or know some shortcut.

The former doesn't work well because you can have,

```sage: f = huge_expression1 + huge_expression2
sage: len(f)
2
```

The latter works comparably to the string length, but was slow as dirt (my implementation is lame). It could probably be improved, though. I would definitely include this if some pynac magic can speed it up.

comment:11 follow-up: ↓ 12 Changed 10 years ago by kcrisman

Perhaps it would help to abstract out something. From discussing with Dan, it sounds like you actually agree more than you think.

• Those who want a command that simplifies definitely want the current version, or at least something like it.
• Those who use something with the word `rectform` in it will expect something with right angles.

Could we have two functions, then?

1. Something with `simplify_foo` where `foo` is something that implies `e^(pi*i*n)` will look very simple with `n` an integer but `e^(i*x)` doesn't change?
2. Something that allows people to access `rectform` without using `maxima_methods`? (I think that the group of folks who would like this is not insignificant, since a lot of people don't know Maxima when coming to Sage but definitely want this sort of symbolic stuff.)

Thoughts are welcome.

comment:12 in reply to: ↑ 11 Changed 10 years ago by mjo

Perhaps it would help to abstract out something. From discussing with Dan, it sounds like you actually agree more than you think.

• Those who want a command that simplifies definitely want the current version, or at least something like it.
• Those who use something with the word `rectform` in it will expect something with right angles.

Could we have two functions, then?

1. Something with `simplify_foo` where `foo` is something that implies `e^(pi*i*n)` will look very simple with `n` an integer but `e^(i*x)` doesn't change?
2. Something that allows people to access `rectform` without using `maxima_methods`? (I think that the group of folks who would like this is not insignificant, since a lot of people don't know Maxima when coming to Sage but definitely want this sort of symbolic stuff.)

Thoughts are welcome.

Sure, I'll create another ticket for adding `Expression.rectform` (that one should be an easy review) and implement `simplify_rectform()` in terms of that. I'll try to do it later today.

comment:13 Changed 10 years ago by mjo

• Dependencies set to #13061

There's a patch up at #13061 adding plain `rectform()`, so I'll rebase this one off that.

Changed 10 years ago by mjo

Updated patch based on Expression.rectform()

comment:14 Changed 10 years ago by mjo

• Status changed from needs_work to needs_review

Ok, patch is updated.

comment:16 Changed 10 years ago by mjo

• Authors set to Michael Orlitzky

comment:17 Changed 9 years ago by jdemeyer

• Milestone changed from sage-5.11 to sage-5.12

comment:18 Changed 9 years ago by vbraun_spam

• Milestone changed from sage-6.1 to sage-6.2

comment:19 Changed 8 years ago by rws

• Branch set to u/rws/ticket/11785
• Modified changed from 01/30/14 21:20:52 to 01/30/14 21:20:52

comment:20 Changed 8 years ago by rws

• Commit set to b2001c09f8407c6b256e7d49e67e449ef8cbc78c
• Reviewers set to Ralf Stephan
• Status changed from needs_review to positive_review

Rebased and changed doctest output order so it tests OK.

New commits:

 ​accb798 `Trac #11785: Provide an interface to Maxima's rectform().` ​b2001c0 `trac #11785: reviewers patch: change doctest expected string ordering`

comment:21 Changed 8 years ago by git

• Commit changed from b2001c09f8407c6b256e7d49e67e449ef8cbc78c to 2fb52ed22e39c63908bb612bb91ca9fd4126f477
• Status changed from positive_review to needs_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

 ​2fb52ed `Merge branch 'develop' into ticket/11785`

comment:22 Changed 8 years ago by rws

• Status changed from needs_review to positive_review

comment:23 Changed 8 years ago by vbraun

• Branch changed from u/rws/ticket/11785 to 2fb52ed22e39c63908bb612bb91ca9fd4126f477
• Resolution set to fixed
• Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.