Opened 8 years ago

Closed 6 years ago

#11785 closed enhancement (fixed)

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

Reported by: ddrake Owned by: burcin
Priority: major Milestone: sage-6.2
Component: symbolics Keywords: maxima symbolics exponentials sd40.5
Cc: kcrisman, mjo Merged in:
Authors: Michael Orlitzky Reviewers: Ralf Stephan
Report Upstream: N/A Work issues:
Branch: 2fb52ed (Commits) Commit: 2fb52ed22e39c63908bb612bb91ca9fd4126f477
Dependencies: #13061 Stopgaps:

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.

Attachments (1)

sage-trac_11785.patch (5.4 KB) - added by mjo 8 years ago.
Updated patch based on Expression.rectform()

Download all attachments as: .zip

Change History (24)

comment:1 Changed 8 years ago by kcrisman

  • Cc kcrisman added

comment:2 Changed 8 years ago by mjo

  • Cc mjo added
  • 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 8 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 8 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 8 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 8 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 8 years ago by davidloeffler

  • Status changed from needs_review to needs_work

comment:8 Changed 8 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: Changed 8 years ago by ddrake

  • Keywords sd40.5 added
  • 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 8 years ago by mjo

Replying to ddrake:

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: Changed 8 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 8 years ago by mjo

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

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 8 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 8 years ago by mjo

Updated patch based on Expression.rectform()

comment:14 Changed 8 years ago by mjo

  • Status changed from needs_work to needs_review

Ok, patch is updated.

comment:15 Changed 7 years ago by jdemeyer

Please fill in your real name as Author.

comment:16 Changed 7 years ago by mjo

  • Authors set to Michael Orlitzky

comment:17 Changed 6 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:18 Changed 6 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:19 Changed 6 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 6 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:

accb798Trac #11785: Provide an interface to Maxima's rectform().
b2001c0trac #11785: reviewers patch: change doctest expected string ordering

comment:21 Changed 6 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:

2fb52edMerge branch 'develop' into ticket/11785

comment:22 Changed 6 years ago by rws

  • Status changed from needs_review to positive_review

comment:23 Changed 6 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.