expansion refactoring

11 views
Skip to first unread message

Ondrej Certik

unread,
Jun 10, 2009, 1:01:43 AM6/10/09
to sympy
Hi,

Aaron has done a great job in refactoring the current expand() methods
all over sympy, so that exponentials are not automatically combined
and expanded, unless you tell sympy to do so, more details here:

http://code.google.com/p/sympy/issues/detail?id=1455

basically we need to distinguish between expanding things like
(x+y)**2, expanding exponentials exp(x+y) and expanding logarithms
log(x*y), so we are discussing the best interface for it, see the
issue.

Any comments are welcome.

Ondrej

Aaron S. Meurer

unread,
Jun 10, 2009, 1:45:52 AM6/10/09
to sy...@googlegroups.com
So Ondrej and I were talking on #sympy, and we came up with the
following naming scheme for expand. The following would be
implemented in their respective classes as _eval_expand_<hint> as it
is currently done, and would be called as expand(expr, **hints), where
hints is a dictionary of things like trig=True which would activate
each expanding method.

Here are the proposed hints:
* trig - This is already implemented.
* complex - Also already implemented.
* func - Also already implemented. I won't touch these three.
* power_exp - This would expand exp(x+y) to exp(x)*exp(y) and 2**(x+y)
to 2**x*2**y. The reverse of powsimp(combine='exp') as written in my
patch for issue 252.
* power_base - (a*b)**x to a**x*b**x. The reverse of
powsimp(combine='base') in my issue 252 patches.
* multinomial - (x+y+...)**n where n is a positive integer to its
multinomial expansion, i.e., a**n + n*a**(n-1)*b + ...
* log - Pull powers out of logs. log(x**2) to 2*log(x)
* mul - Distribute multiplication over addition. x*(y+z) to x*y+x*z

We will be getting rid of expand_basic, which was default. power_exp,
power_base, multinomial, log, and mul will be on by default. These
represent the old behavior of expand_basic. We would also delete the
old defunct expand_power which was never implemented anywhere anyway.

Aaron Meurer

Fabian Pedregosa

unread,
Jun 10, 2009, 2:09:51 AM6/10/09
to sy...@googlegroups.com
Aaron S. Meurer wrote:
> So Ondrej and I were talking on #sympy, and we came up with the
> following naming scheme for expand. The following would be
> implemented in their respective classes as _eval_expand_<hint> as it
> is currently done, and would be called as expand(expr, **hints), where
> hints is a dictionary of things like trig=True which would activate
> each expanding method.
>

The naming for expand is nice, but as for all those ._eval_expand_*
methods, I think they just bloat the object system. A resolution method
like the one used by the printer (outside the classes) would be nicer IMHO.
--
http://fseoane.net/blog/

Aaron S. Meurer

unread,
Jun 10, 2009, 12:05:50 PM6/10/09
to sy...@googlegroups.com
I was thinking that one problem with the naming system that I proposed
below is that if you want to, for example, only expand logs, you have
to do

>>> expand(expr, mul=False, power_base=False, multinomial=False,
power_exp=False)

It should be easier to just do one. Any ideas? Maybe an only key
that would override all of the other keys, like

>>> expand(expr, only=log) # Automatically makes mul, power_base,
multinomial, and power_exp False


Fabian, could you explain how this resolution method would work. I am
not familiar with the printer.

Aaron Meurer

Fabian Pedregosa

unread,
Jun 10, 2009, 12:38:15 PM6/10/09
to sy...@googlegroups.com
Aaron S. Meurer wrote:
> I was thinking that one problem with the naming system that I proposed
> below is that if you want to, for example, only expand logs, you have
> to do
>
> >>> expand(expr, mul=False, power_base=False, multinomial=False,
> power_exp=False)
>
> It should be easier to just do one. Any ideas? Maybe an only key
> that would override all of the other keys, like
>
> >>> expand(expr, only=log) # Automatically makes mul, power_base,
> multinomial, and power_exp False
>
>
> Fabian, could you explain how this resolution method would work. I am
> not familiar with the printer.

you would have a class (Say Expand) with methods
._expand_<hint>_<class>, where <class> is the expr's class, and those
methods do the equivalent job of <class>._eval_expand_<hint>. Or even
better, you could have classes Expand<hint>, which methods ._expand_<hint>

It has the benefits that it does not make core classes grow huge since
all code is externalized and all expand code is kept in a single place,
not scattered across multiple classes.
--
http://fseoane.net/blog/

Ondrej Certik

unread,
Jun 10, 2009, 12:54:55 PM6/10/09
to sy...@googlegroups.com
On Wed, Jun 10, 2009 at 10:38 AM, Fabian Pedregosa<fab...@fseoane.net> wrote:
>
> Aaron S. Meurer wrote:
>> I was thinking that one problem with the naming system that I proposed
>> below is that if you want to, for example, only expand logs, you have
>> to do
>>
>>  >>> expand(expr, mul=False, power_base=False, multinomial=False,
>> power_exp=False)
>>
>> It should be easier to just do one.  Any ideas?   Maybe an only key
>> that would override all of the other keys, like
>>
>>  >>> expand(expr, only=log) # Automatically makes mul, power_base,
>> multinomial, and power_exp False

We can also have a function expand_log(), that would do the above. I
think in fact one could need quite often to just expand logs or trigs,
but not powers.

>>
>>
>> Fabian, could you explain how this resolution method would work.  I am
>> not familiar with the printer.
>
> you would have a class (Say Expand) with methods
> ._expand_<hint>_<class>, where <class> is the expr's class, and those
> methods do the equivalent job of <class>._eval_expand_<hint>. Or even
> better, you could have classes Expand<hint>, which methods ._expand_<hint>
>
> It has the benefits that it does not make core classes grow huge since
> all code is externalized and all expand code is kept in a single place,
> not scattered across multiple classes.

Nice idea. I would do that after Aaron patches are in. Currently we
already have those methods anyways, so let's fix it first and then
refactor it to Expand.

We can do the same for Series. And Limits. Lots of algorithms need
some special code for each of our core classes. In limits, it would be
an overkill though, e.g. here is an example that dispatches for each
sympy class:

http://git.sympy.org/?p=sympy.git;a=blob;f=sympy/series/gruntz.py;h=bf5d82ce498b66d567100d6471d83e0bde38690b;hb=HEAD#l196

Ondrej

Aaron S. Meurer

unread,
Jun 10, 2009, 1:47:39 PM6/10/09
to sy...@googlegroups.com

On Jun 10, 2009, at 10:54 AM, Ondrej Certik wrote:

>
> On Wed, Jun 10, 2009 at 10:38 AM, Fabian
> Pedregosa<fab...@fseoane.net> wrote:
>>
>> Aaron S. Meurer wrote:
>>> I was thinking that one problem with the naming system that I
>>> proposed
>>> below is that if you want to, for example, only expand logs, you
>>> have
>>> to do
>>>
>>> >>> expand(expr, mul=False, power_base=False, multinomial=False,
>>> power_exp=False)
>>>
>>> It should be easier to just do one. Any ideas? Maybe an only key
>>> that would override all of the other keys, like
>>>
>>> >>> expand(expr, only=log) # Automatically makes mul, power_base,
>>> multinomial, and power_exp False
>
> We can also have a function expand_log(), that would do the above. I
> think in fact one could need quite often to just expand logs or trigs,
> but not powers.
>

Also expansion of just mul. I will create expand_log() and
expand_mul() functions. And of course, I will document well in the
expand() function how to get just one behavior.

>>>
>>>
>>> Fabian, could you explain how this resolution method would work.
>>> I am
>>> not familiar with the printer.
>>
>> you would have a class (Say Expand) with methods
>> ._expand_<hint>_<class>, where <class> is the expr's class, and those
>> methods do the equivalent job of <class>._eval_expand_<hint>. Or even
>> better, you could have classes Expand<hint>, which
>> methods ._expand_<hint>
>>
>> It has the benefits that it does not make core classes grow huge
>> since
>> all code is externalized and all expand code is kept in a single
>> place,
>> not scattered across multiple classes.
>
> Nice idea. I would do that after Aaron patches are in. Currently we
> already have those methods anyways, so let's fix it first and then
> refactor it to Expand.

OK. I will go ahead and do it the way that we originally planned.


>
> We can do the same for Series. And Limits. Lots of algorithms need
> some special code for each of our core classes. In limits, it would be
> an overkill though, e.g. here is an example that dispatches for each
> sympy class:
>
> http://git.sympy.org/?p=sympy.git;a=blob;f=sympy/series/gruntz.py;h=bf5d82ce498b66d567100d6471d83e0bde38690b;hb=HEAD#l196
>
> Ondrej

Aaron Meurer

Ondrej Certik

unread,
Jun 10, 2009, 2:04:14 PM6/10/09
to sy...@googlegroups.com
On Wed, Jun 10, 2009 at 11:47 AM, Aaron S. Meurer<asme...@gmail.com> wrote:
>
>
> On Jun 10, 2009, at 10:54 AM, Ondrej Certik wrote:
>
>>
>> On Wed, Jun 10, 2009 at 10:38 AM, Fabian
>> Pedregosa<fab...@fseoane.net> wrote:
>>>
>>> Aaron S. Meurer wrote:
>>>> I was thinking that one problem with the naming system that I
>>>> proposed
>>>> below is that if you want to, for example, only expand logs, you
>>>> have
>>>> to do
>>>>
>>>>  >>> expand(expr, mul=False, power_base=False, multinomial=False,
>>>> power_exp=False)
>>>>
>>>> It should be easier to just do one.  Any ideas?   Maybe an only key
>>>> that would override all of the other keys, like
>>>>
>>>>  >>> expand(expr, only=log) # Automatically makes mul, power_base,
>>>> multinomial, and power_exp False
>>
>> We can also have a function expand_log(), that would do the above. I
>> think in fact one could need quite often to just expand logs or trigs,
>> but not powers.
>>
> Also expansion of just mul.  I will create expand_log() and
> expand_mul() functions.  And of course, I will document well in the
> expand() function how to get just one behavior.

Sounds good.

>
>>>>
>>>>
>>>> Fabian, could you explain how this resolution method would work.
>>>> I am
>>>> not familiar with the printer.
>>>
>>> you would have a class (Say Expand) with methods
>>> ._expand_<hint>_<class>, where <class> is the expr's class, and those
>>> methods do the equivalent job of <class>._eval_expand_<hint>. Or even
>>> better, you could have classes Expand<hint>, which
>>> methods ._expand_<hint>
>>>
>>> It has the benefits that it does not make core classes grow huge
>>> since
>>> all code is externalized and all expand code is kept in a single
>>> place,
>>> not scattered across multiple classes.
>>
>> Nice idea. I would do that after Aaron patches are in. Currently we
>> already have those methods anyways, so let's fix it first and then
>> refactor it to Expand.
> OK.  I will go ahead and do it the way that we originally planned.

Yep. Later when things work, we can refactor it. I'll be working on
the cython core later in the summer, when we get rid of the current
assumptions, so we might have some other design decisions too.

Ondrej

Brian Granger

unread,
Jun 10, 2009, 3:32:41 PM6/10/09
to sy...@googlegroups.com
> you would have a class (Say Expand) with methods
> ._expand_<hint>_<class>, where <class> is the expr's class, and those
> methods do the equivalent job of <class>._eval_expand_<hint>. Or even
> better, you could have classes Expand<hint>, which methods ._expand_<hint>
>
> It has the benefits that it does not make core classes grow huge since
> all code is externalized and all expand code is kept in a single place,
> not scattered across multiple classes.

Doesn't this violate the core concept of an object, namely
encapsulation? I find it very confusing when I start to have to look
all over the code base to find the functionality of a single sympy
type. It also makes a system that is very magic, hard to debug as the
execution path jumps all over the place between different objects.

If I understand Expand correctly, Printing is a very different. With
Printing, we want users to be able to create new printers and create
their own Sympy types that know how to be printed with existing
printers. And we pay a price of this flexibilty as the printing logic
is difficult to follow and maintain.

The above proposal makes it impossible for users to create their own
Sympy types that have expand logic, unless they modify the Sympy core.
That is a bad idea.

Thus, I am -1 on having a global Expand class for all Sympy types.

I would be fine with each Sympy class having its own associated Expand
class. That somewhat separates the expand logic from the core class,
but doesn't put all the expand logic for all of Sympy in one place.
Something like:

class Mul(...):
...

class ExpandMul(BaseExpand):
...

Here is another way of saying this...

* If I want to work on a particular Sympy class, such as Sum (picking
one at random). I want all the logic for Sum to be in *one* file
(could be many classes in that file though). And, I don't want logic
from other Sympy types poluting that file or those classes.

* If a user wants to create their own fully functional Sympy type.
They should be able to put all the logic in *one* file and not have to
touch exiting Sympy code.

Cheers,

Brian

smichr

unread,
Jun 10, 2009, 3:48:28 PM6/10/09
to sympy
> It should be easier to just do one. Any ideas? Maybe an only key
> that would override all of the other keys, like
>
> >>> expand(expr, only=log) # Automatically makes mul, power_base,
> multinomial, and power_exp False

How about a parameter "all" that has True or False which gets
evaluated before any other settings which turns off or on all settings
and then you set what you want:

>>> expand(expr, all=False, log=True, power_exp=True)

/c

Ondrej Certik

unread,
Jun 10, 2009, 3:54:47 PM6/10/09
to sy...@googlegroups.com
On Wed, Jun 10, 2009 at 1:32 PM, Brian Granger<elliso...@gmail.com> wrote:
>
>> you would have a class (Say Expand) with methods
>> ._expand_<hint>_<class>, where <class> is the expr's class, and those
>> methods do the equivalent job of <class>._eval_expand_<hint>. Or even
>> better, you could have classes Expand<hint>, which methods ._expand_<hint>
>>
>> It has the benefits that it does not make core classes grow huge since
>> all code is externalized and all expand code is kept in a single place,
>> not scattered across multiple classes.
>
> Doesn't this violate the core concept of an object, namely
> encapsulation?  I find it very confusing when I start to have to look
> all over the code base to find the functionality of a single sympy
> type.  It also makes a system that is very magic, hard to debug as the
> execution path jumps all over the place between different objects.

Yes and no. For example for the series expansion, currently this is
implemented in the _eval_nseries() methods, which jump from class to
class, as you recursively expand things. So there is a value in having
all series expansion in one file.

So far I really sticked to the principle that functionality related to
one object should be *in that object*, but in the printer case, it was
just too messy and unacceptable anymore.

>
> If I understand Expand correctly, Printing is a very different.  With
> Printing, we want users to be able to create new printers and create
> their own Sympy types that know how to be printed with existing
> printers.  And we pay a price of this flexibilty as the printing logic
> is difficult to follow and maintain.

In fact, I think it's actually much easier to maintain now, since all
the printing logic is in *one* file (one file per printer, e.g. one
file for pretty printing, one file for latex, etc.).

But you are right that Expand is different, there is usually just one
way to expand things, no need to have prettyexpand, or latexexpand.

>
> The above proposal makes it impossible for users to create their own
> Sympy types that have expand logic, unless they modify the Sympy core.
>  That is a bad idea.

That is a very good point. One would have to have means to be able to
override things in the new class, just like the _latex_ and similar
methods for printing. It would make things messier, probably, but
maybe if done correctly, it could be easier to follow.

>
> Thus, I am -1 on having a global Expand class for all Sympy types.
>
> I would be fine with each Sympy class having its own associated Expand
> class.  That somewhat separates the expand logic from the core class,
> but doesn't put all the expand logic for all of Sympy in one place.
> Something like:
>
> class Mul(...):
>    ...
>
> class ExpandMul(BaseExpand):
>    ...

I think this is the same as just using one class.

>
> Here is another way of saying this...
>
> * If I want to work on a particular Sympy class, such as Sum (picking
> one at random).  I want all the logic for Sum to be in *one* file
> (could be many classes in that file though).  And, I don't want logic
> from other Sympy types poluting that file or those classes.
>
> * If a user wants to create their own fully functional Sympy type.
> They should be able to put all the logic in *one* file and not have to
> touch exiting Sympy code.

+1, those are the main goals.

That if you create a new class (=new sympy type), you just override
couple methods and things will start working. And it is still like
that in sympy (even with printing).


Ondrej

Brian Granger

unread,
Jun 10, 2009, 4:45:52 PM6/10/09
to sy...@googlegroups.com
>> Doesn't this violate the core concept of an object, namely
>> encapsulation?  I find it very confusing when I start to have to look
>> all over the code base to find the functionality of a single sympy
>> type.  It also makes a system that is very magic, hard to debug as the
>> execution path jumps all over the place between different objects.
>
> Yes and no. For example for the series expansion, currently this is
> implemented in the _eval_nseries() methods, which jump from class to
> class, as you recursively expand things. So there is a value in having
> all series expansion in one file.

So you think the encapsulation of expand related things is more
important than encapsulation of sympy types. This is, I guess, a form
of encapsulation.

>> The above proposal makes it impossible for users to create their own
>> Sympy types that have expand logic, unless they modify the Sympy core.
>>  That is a bad idea.
>
> That is a very good point.  One would have to have means to be able to
> override things in the new class, just like the _latex_ and similar
> methods for printing. It would make things messier, probably, but
> maybe if done correctly, it could be easier to follow.

So, there are a couple of options I guess:

1. Make all expand methods related to class Foo methods of the class
itself. This is simple and allows user defined types.

2. Make all expand method part of one global Expand class. This is
simple, but doesn't allow user defined types.

3. A hybrid approach like the Printer uses. This is more complex, but
allows user defined types, but keeps the expand logic of the core
sympy types together.

I prefer 1, then 3 and then 2.

>> class Mul(...):
>>    ...
>>
>> class ExpandMul(BaseExpand):
>>    ...
>
> I think this is the same as just using one class.

Not from the perspective of creating custom user defined types. With
this approach, a user doesn't have to modify the global Expand class.
They just create their own Expand classes for their types and it
works.

>> * If I want to work on a particular Sympy class, such as Sum (picking
>> one at random).  I want all the logic for Sum to be in *one* file
>> (could be many classes in that file though).  And, I don't want logic
>> from other Sympy types poluting that file or those classes.
>>
>> * If a user wants to create their own fully functional Sympy type.
>> They should be able to put all the logic in *one* file and not have to
>> touch exiting Sympy code.
>
> +1, those are the main goals.

Great. This is important to me because there are number of projects I
want to use Sympy on, and in each case, I would need to create many
new Sympy types.

Cheers,

Brian

Ondrej Certik

unread,
Jun 10, 2009, 5:42:15 PM6/10/09
to sy...@googlegroups.com
On Wed, Jun 10, 2009 at 2:45 PM, Brian Granger<elliso...@gmail.com> wrote:
>
>>> Doesn't this violate the core concept of an object, namely
>>> encapsulation?  I find it very confusing when I start to have to look
>>> all over the code base to find the functionality of a single sympy
>>> type.  It also makes a system that is very magic, hard to debug as the
>>> execution path jumps all over the place between different objects.
>>
>> Yes and no. For example for the series expansion, currently this is
>> implemented in the _eval_nseries() methods, which jump from class to
>> class, as you recursively expand things. So there is a value in having
>> all series expansion in one file.
>
> So you think the encapsulation of expand related things is more
> important than encapsulation of sympy types.  This is, I guess, a form
> of encapsulation.

No, what I wanted to say is that I like 1), then 3) then 2), from the
options below.

For printing I think 3) is the best. For all the other things so far
1) was the best, because it's simple and it gets the job done.

>
>>> The above proposal makes it impossible for users to create their own
>>> Sympy types that have expand logic, unless they modify the Sympy core.
>>>  That is a bad idea.
>>
>> That is a very good point.  One would have to have means to be able to
>> override things in the new class, just like the _latex_ and similar
>> methods for printing. It would make things messier, probably, but
>> maybe if done correctly, it could be easier to follow.
>
> So, there are a couple of options I guess:
>
> 1. Make all expand methods related to class Foo methods of the class
> itself.  This is simple and allows user defined types.

But it doesn't allow user defined functionality on sympy core objects.
E.g. if I want to implement my new shiny printer, I would have to
either monkey patch sympy core classes (ugly), or write my new printer
just like the current sympy printers (e.g. approach 3), thus I will be
a second class citizen, because I cannot easily leverage what's
already in sympy.

>
> 2. Make all expand method part of one global Expand class.  This is
> simple, but doesn't allow user defined types.

Yes.

>
> 3. A hybrid approach like the Printer uses.  This is more complex, but
> allows user defined types, but keeps the expand logic of the core
> sympy types together.

Yes.

>
> I prefer 1, then 3 and then 2.

The same here.

>
>>> class Mul(...):
>>>    ...
>>>
>>> class ExpandMul(BaseExpand):
>>>    ...
>>
>> I think this is the same as just using one class.
>
> Not from the perspective of creating custom user defined types.  With
> this approach,  a user doesn't have to modify the global Expand class.
>  They just create their own Expand classes for their types and it
> works.

I meant that this is the same as the case 1).

>
>>> * If I want to work on a particular Sympy class, such as Sum (picking
>>> one at random).  I want all the logic for Sum to be in *one* file
>>> (could be many classes in that file though).  And, I don't want logic
>>> from other Sympy types poluting that file or those classes.
>>>
>>> * If a user wants to create their own fully functional Sympy type.
>>> They should be able to put all the logic in *one* file and not have to
>>> touch exiting Sympy code.
>>
>> +1, those are the main goals.
>
> Great.  This is important to me because there are number of projects I
> want to use Sympy on, and in each case, I would need to create many
> new Sympy types.

Yes. I'll be glad to help you with all those projects.

Ondrej

Fabian Pedregosa

unread,
Jun 10, 2009, 6:30:44 PM6/10/09
to sy...@googlegroups.com
Ondrej Certik wrote:
> On Wed, Jun 10, 2009 at 2:45 PM, Brian Granger<elliso...@gmail.com> wrote:
>>>> Doesn't this violate the core concept of an object, namely
>>>> encapsulation? I find it very confusing when I start to have to look
>>>> all over the code base to find the functionality of a single sympy
>>>> type. It also makes a system that is very magic, hard to debug as the
>>>> execution path jumps all over the place between different objects.
>>> Yes and no. For example for the series expansion, currently this is
>>> implemented in the _eval_nseries() methods, which jump from class to
>>> class, as you recursively expand things. So there is a value in having
>>> all series expansion in one file.
>> So you think the encapsulation of expand related things is more
>> important than encapsulation of sympy types. This is, I guess, a form
>> of encapsulation.
>
> No, what I wanted to say is that I like 1), then 3) then 2), from the
> options below.
>
> For printing I think 3) is the best. For all the other things so far
> 1) was the best, because it's simple and it gets the job done.
>
>>>> The above proposal makes it impossible for users to create their own
>>>> Sympy types that have expand logic, unless they modify the Sympy core.
>>>> That is a bad idea.

In my oppinion current system also suffers from this. Imagine you define
a non-associative-symbol:

class NAS(Symbol): # NAS = Non-associative-symbol
...

and then I do, (x, y are instances of NAS):

>>> expr = (x+y)**2 + x

then type(expr) would be Add. If I called expr.expand(), it would do it
*wrong* , because the .expand() on Add knows nothing about
non-associativity.

i.e., to implement new behavior you have to rewrite the ._eval_expand on
some of the core classes, so I do not quite agree on the point below:
"...allows user defined types"

>>> That is a very good point. One would have to have means to be able to
>>> override things in the new class, just like the _latex_ and similar
>>> methods for printing. It would make things messier, probably, but
>>> maybe if done correctly, it could be easier to follow.
>> So, there are a couple of options I guess:
>>
>> 1. Make all expand methods related to class Foo methods of the class
>> itself. This is simple and allows user defined types.
>
> But it doesn't allow user defined functionality on sympy core objects.
> E.g. if I want to implement my new shiny printer, I would have to
> either monkey patch sympy core classes (ugly), or write my new printer
> just like the current sympy printers (e.g. approach 3), thus I will be
> a second class citizen, because I cannot easily leverage what's
> already in sympy.
>
>> 2. Make all expand method part of one global Expand class. This is
>> simple, but doesn't allow user defined types.
>
> Yes.
>
>> 3. A hybrid approach like the Printer uses. This is more complex, but
>> allows user defined types, but keeps the expand logic of the core
>> sympy types together.


For expansion, I think 3 is the best. And it has another advantage. You
don't load code that you don't use. In (1), having to override at least
5 methods in each class (not to mention expand code will probably grow a
lot in the future) will make every class in sympy just even heavier,
whether you use expand in your code, or not.
--
http://fseoane.net/blog/

Brian Granger

unread,
Jun 10, 2009, 6:57:29 PM6/10/09
to sy...@googlegroups.com
> No, what I wanted to say is that I like 1), then 3) then 2), from the
> options below.
>
> For printing I think 3) is the best. For all the other things so far
> 1) was the best, because it's simple and it gets the job done.

Ahh, OK, I misunderstood you.

>> 1. Make all expand methods related to class Foo methods of the class
>> itself.  This is simple and allows user defined types.
>
> But it doesn't allow user defined functionality on sympy core objects.
> E.g. if I want to implement my new shiny printer, I would have to
> either monkey patch sympy core classes (ugly), or write my new printer
> just like the current sympy printers (e.g. approach 3), thus I will be
> a second class citizen, because I cannot easily leverage what's
> already in sympy.

Yes, I hadn't thought of this. I guess I am mostly focused on
non-sympy core objects, but you bring up a really good case as well.
But are you saying you are willing to live with this limitation of
(1)?

>> I prefer 1, then 3 and then 2.
>
> The same here.

OK, we agree then.

But, I thought the original idea was to make a single Expand class for
all all core types, which in my mind is option (2)? Maybe I am
confused...

I think we agree though.

Cheers,

Brian

Ondrej Certik

unread,
Jun 10, 2009, 7:02:59 PM6/10/09
to sy...@googlegroups.com
On Wed, Jun 10, 2009 at 4:57 PM, Brian Granger<elliso...@gmail.com> wrote:
>
>> No, what I wanted to say is that I like 1), then 3) then 2), from the
>> options below.
>>
>> For printing I think 3) is the best. For all the other things so far
>> 1) was the best, because it's simple and it gets the job done.
>
> Ahh, OK, I misunderstood you.
>
>>> 1. Make all expand methods related to class Foo methods of the class
>>> itself.  This is simple and allows user defined types.
>>
>> But it doesn't allow user defined functionality on sympy core objects.
>> E.g. if I want to implement my new shiny printer, I would have to
>> either monkey patch sympy core classes (ugly), or write my new printer
>> just like the current sympy printers (e.g. approach 3), thus I will be
>> a second class citizen, because I cannot easily leverage what's
>> already in sympy.
>
> Yes, I hadn't thought of this.  I guess I am mostly focused on
> non-sympy core objects, but you bring up a really good case as well.
> But are you saying you are willing to live with this limitation of
> (1)?

Only as long as the user doesn't need to extend the algorithm. As you
can see, for printing we switched to (3) and it works great, several
people have already customized the printers, which would be nearly
impossible with (1).

>
>>> I prefer 1, then 3 and then 2.
>>
>> The same here.
>
> OK, we agree then.
>
> But, I thought the original idea was to make a single Expand class for
> all all core types, which in my mind is option (2)?  Maybe I am
> confused...

Yes, and I was wrong. For Expand we should first stay with (1), then
possibly refactor to (3), but never to (2).

> I think we agree though.

Yep.

Ondrej

Brian Granger

unread,
Jun 10, 2009, 7:07:04 PM6/10/09
to sy...@googlegroups.com
> Only as long as the user doesn't need to extend the algorithm. As you
> can see, for printing we switched to (3) and it works great, several
> people have already customized the printers, which would be nearly
> impossible with (1).

Yes, definitely.

>> But, I thought the original idea was to make a single Expand class for
>> all all core types, which in my mind is option (2)?  Maybe I am
>> confused...
>
> Yes, and I was wrong. For Expand we should first stay with (1), then
> possibly refactor to (3), but never to (2).

OK great.

Cheers,

Brian

Aaron S. Meurer

unread,
Jun 10, 2009, 10:58:58 PM6/10/09
to sy...@googlegroups.com
How stringent should expand_log be with assumptions? Technically,
log(x**2) == 2*log(x) is only true if x is positive real. For example:
>>> log((-1)**2)
0
>>> 2*log(-1)
2⋅π⋅ⅈ
Currently, it works no matter what assumptions are on x, as long as
the exponent is a number (this should actually be if the exponent is
real). Also, log(x*y) => log(x) + log(y) currently works only if x*y
is real, though it actually requires that x*y is positive (confer
above, with x=y=-1). I could easily make it only work with the right
assumptions on the variables, but that could be a burden on people
(and code) that just wants log(x**2) to expand to 2*log(x) without
setting x to positive.

I think logs are the only expansion that requires proper assumptions
for it to work. By the way, in the logcombine function that I wrote
(which hopefully will make it in one of these days), I made it only
work with the proper assumptions but also added an assume_pos_real
keyword argument which could override them. (logcombine does the
opposite of expand, 2*log(x) => log(x**2) and log(x) + log(y) =>
log(x*y) ).

Aaron Meurer

Vinzent Steinberg

unread,
Jun 11, 2009, 5:20:19 AM6/11/09
to sympy
On Jun 10, 8:09 am, Fabian Pedregosa <fab...@fseoane.net> wrote:
> The naming for expand is nice, but as for all those ._eval_expand_*
> methods, I think they just bloat the object system. A resolution method
> like the one used by the printer (outside the classes) would be nicer IMHO.

I don't think they pollute the namespace, because they are private
(beginning underscore).
However, it would be trivial to change the code from _eval_expand_hint
to _eval_expand(hint).
But this would make overloading more difficult.

Vinzent

Vinzent Steinberg

unread,
Jun 11, 2009, 5:23:36 AM6/11/09
to sympy
I'd add a flag to force this expansion (ignoring assumptions), like
you did for logcombine. By default it should not be expanded imho.

Vinzent

Aaron S. Meurer

unread,
Jun 11, 2009, 12:18:40 PM6/11/09
to sy...@googlegroups.com
The problem with adding a flag is that I would have to put it in the
global expand function, which is perhaps too special of consideration
in what is supposed to be a modular function. I think a better option
would be to have a separate _expand_log_all function that ignores
assumptions difficulties. By the way, what is a better name than
log_all for this function?

Aaron Meurer

Vinzent Steinberg

unread,
Jun 12, 2009, 5:11:29 AM6/12/09
to sympy
On Jun 11, 6:18 pm, "Aaron S. Meurer" <asmeu...@gmail.com> wrote:
> The problem with adding a flag is that I would have to put it in the  
> global expand function, which is perhaps too special of consideration  
> in what is supposed to be a modular function.

For now we can have assume_positive, later we can add a more generale
assume=x>0 (or similar) flag.

> I think a better option  
> would be to have a separate _expand_log_all function that ignores  
> assumptions difficulties.  By the way, what is a better name than  
> log_all for this function?

I don't see the advantage of this.


Vinzent

Aaron S. Meurer

unread,
Jun 14, 2009, 11:12:32 PM6/14/09
to sy...@googlegroups.com
While working on this, I came across this in the tests:

assert polygamma(2, 3*x).expand(func=True) == polygamma(2, 3*x)/9

I do not know anything about polygamma other than the definition, but
it this seems like a strange identity for any function, namely, x == x/
9. I plugged it into WolframAlpha and got what I expected, that it is
only equal when polygamma(2, 3*x) == 0 (http://www76.wolframalpha.com/input/?i=polygamma%282%2C+3*x%29+%3D%3D+polygamma%282%2C+3*x%29%2F9
).

Could someone please enlighten me about this? Does expand(polygamma)
not have the same meaning as expand(everything else), namely, that it
returns the same function rewritten in a larger way somehow? If so,
then it need to have a different name than expand. Otherwise, how
does polygamma(2, 3*x) == polygamma(2, 3*x)/9?

The reason I am asking is that it is causing infinite recursion
problems (as you could imagine, the function could expand like that
forever).

Aaron Meurer

Vinzent Steinberg

unread,
Jun 15, 2009, 4:46:54 AM6/15/09
to sympy
On Jun 15, 5:12 am, "Aaron S. Meurer" <asmeu...@gmail.com> wrote:
> While working on this, I came across this in the tests:
>
> assert polygamma(2, 3*x).expand(func=True) == polygamma(2, 3*x)/9

Even if this was true (which it obviously isn't for any polygamma
(x,y) != 0), it would imho make no sense to expand it to something
more complex without giving any advantage.

Vinzent

Aaron S. Meurer

unread,
Jun 17, 2009, 1:30:38 AM6/17/09
to sy...@googlegroups.com
I have a patch worked up. Please review. See http://code.google.com/p/sympy/issues/detail?can=2&start=0&num=100&q=asmeurer&colspec=ID%20Type%20Status%20Priority%20Milestone%20Owner%20Summary&sort=&id=1455
, or pull from http://github.com/asmeurer/sympy/tree/expand. Here is
the commit message. One thing that I forgot to mention there is that
I had to comment out two polygamma tests that were causing recursion
problems (see earlier posts in this thread):

Refactored expand, fixing issues 1455, 1453, and 1445.
Expand hints are now much more specific. pwoer_exp, power_base,
multinomial, mul,
and log have replaced basic as the default hints. basic remains as
a default hint so
it can be used for things that don't fit into those other hints but
needs to be automatic.

All expand functions include a recursive keyword argument which
determines if it passes
on the expand hints to the inner parts of an expression.

I added expand_mul, expand_log, expand_func, expand_trig, and
expand_complex functions
to make it easier to only expand with those hints.

This also changes a few other things internally. expand_hint
functions no longer return
None if they do not expand. The return the expression. Mul and Add
now have base expand_hint
functions that expands through the terms of the expression. This
replaces similar code
in Basic because not all Basic subclasses works with
class.new(*class.args) (e.g., Poly).

This fixes all the tests that it made fail, either by changing the
test to an equivalent
expression or by changing the expand calling routine in some code
somewhere.

The integration engine relies on expand(log(x**2)) => 2*log(x) working
always, even though
it is technically only true when x is positive. This keeps the old
behavior. Hopefully,
it will be easier to fix when we have the new assumptions system.

Some of this is based on things that I have commited earlier but have
not made it in yet.

In particular, this code is based on my patches in Issue 252, which
keeps exp(x)*exp(y)
from automatically combining to exp(x+y). expand_power_exp won't work
without it.
It also has some stuff in logcombine and maybe a few other places.

Aaron Meurer

Aaron S. Meurer

unread,
Jun 22, 2009, 3:10:04 AM6/22/09
to sy...@googlegroups.com, sympy-...@googlegroups.com
I have a patch worked up for issue 1455 now ready for review. Please
pull from http://github.com/asmeurer/sympy/commits/expand-exp-review.
See also the issue page.

Aaron Meurer

Reply all
Reply to author
Forward
0 new messages