Opened 10 years ago
Closed 7 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, GitHub, GitLab) | 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)
Change History (24)
comment:1 Changed 10 years ago by
- Cc kcrisman added
comment:2 Changed 9 years ago by
- Cc mjo added
- Status changed from new to needs_review
comment:3 Changed 9 years ago by
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 9 years ago by
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 passlambda 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 9 years ago by
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 9 years ago by
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 9 years ago by
- Status changed from needs_review to needs_work
comment:8 Changed 9 years ago by
- 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 9 years ago by
- 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 9 years ago by
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'srectform
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 fromcomplexity_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,
- Know maxima and are familiar with its
rectform()
and don't know about expr.maxima_methods().rectform() and would expectsimplify_rectform()
to just callrectform()
? - 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 9 years ago by
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?
- Something with
simplify_foo
wherefoo
is something that impliese^(pi*i*n)
will look very simple withn
an integer bute^(i*x)
doesn't change? - Something that allows people to access
rectform
without usingmaxima_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 9 years ago by
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?
- Something with
simplify_foo
wherefoo
is something that impliese^(pi*i*n)
will look very simple withn
an integer bute^(i*x)
doesn't change?- Something that allows people to access
rectform
without usingmaxima_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 9 years ago by
- Dependencies set to #13061
There's a patch up at #13061 adding plain rectform()
, so I'll rebase this one off that.
comment:14 Changed 9 years ago by
- Status changed from needs_work to needs_review
Ok, patch is updated.
comment:15 Changed 9 years ago by
Please fill in your real name as Author.
comment:16 Changed 9 years ago by
comment:17 Changed 8 years ago by
- Milestone changed from sage-5.11 to sage-5.12
comment:18 Changed 7 years ago by
- Milestone changed from sage-6.1 to sage-6.2
comment:19 Changed 7 years ago by
- 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 7 years ago by
- Commit set to b2001c09f8407c6b256e7d49e67e449ef8cbc78c
- Reviewers set to Ralf Stephan
- Status changed from needs_review to positive_review
comment:21 Changed 7 years ago by
- 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 7 years ago by
- Status changed from needs_review to positive_review
comment:23 Changed 7 years ago by
- Branch changed from u/rws/ticket/11785 to 2fb52ed22e39c63908bb612bb91ca9fd4126f477
- Resolution set to fixed
- Status changed from positive_review to closed
This is sort of a work-in-progress, but I'd appreciate comments on the approach.