simplify should call doit() on all instances #2936

Open
wants to merge 12 commits into
from

Conversation

Projects
None yet
6 participants
@shiprabanga
Contributor

shiprabanga commented Feb 19, 2014

No description provided.

@skirpichev

This comment has been minimized.

Show comment Hide comment
@skirpichev

skirpichev Feb 19, 2014

Contributor

Why there are *.json files and sympy.py?

Contributor

skirpichev commented Feb 19, 2014

Why there are *.json files and sympy.py?

@asmeurer

This comment has been minimized.

Show comment Hide comment
@asmeurer

asmeurer Feb 20, 2014

Owner

What is the motivation for this? I think there should be only on doit call, in simplify().

Owner

asmeurer commented Feb 20, 2014

What is the motivation for this? I think there should be only on doit call, in simplify().

@shiprabanga

This comment has been minimized.

Show comment Hide comment
@shiprabanga

shiprabanga Feb 20, 2014

Contributor

@skirpichev I removed those. They were included by mistake.

@asmeurer My motivation to add doit() in each of the functions was that each of them are in the simplify module so doit() will reduce whatever expression they return to the highest simplified state. Also , adding it in every function won't affect the right answer and would ensure we get the simplified state for every function.

Contributor

shiprabanga commented Feb 20, 2014

@skirpichev I removed those. They were included by mistake.

@asmeurer My motivation to add doit() in each of the functions was that each of them are in the simplify module so doit() will reduce whatever expression they return to the highest simplified state. Also , adding it in every function won't affect the right answer and would ensure we get the simplified state for every function.

@asmeurer

This comment has been minimized.

Show comment Hide comment
@asmeurer

asmeurer Feb 23, 2014

Owner

No, but it can result in unnecessarily evaluating something, which the user might not want.

Owner

asmeurer commented Feb 23, 2014

No, but it can result in unnecessarily evaluating something, which the user might not want.

@shiprabanga

This comment has been minimized.

Show comment Hide comment
@shiprabanga

shiprabanga Feb 23, 2014

Contributor

Ok so does the latest commit seem fine? I have added doit() only to return values in function simplify.

Contributor

shiprabanga commented Feb 23, 2014

Ok so does the latest commit seem fine? I have added doit() only to return values in function simplify.

@shiprabanga

This comment has been minimized.

Show comment Hide comment
@shiprabanga

shiprabanga Feb 28, 2014

Contributor

Is it failing the Travis build because its now returning a simplified value for the case of simplify function?

Contributor

shiprabanga commented Feb 28, 2014

Is it failing the Travis build because its now returning a simplified value for the case of simplify function?

@asmeurer

This comment has been minimized.

Show comment Hide comment
@asmeurer

asmeurer Mar 23, 2014

Owner

Most of those are unneeded. You really only need the first and last. There is probably a better place to put it in the function, though.

Owner

asmeurer commented Mar 23, 2014

Most of those are unneeded. You really only need the first and last. There is probably a better place to put it in the function, though.

@shiprabanga

This comment has been minimized.

Show comment Hide comment
@shiprabanga

shiprabanga Mar 25, 2014

Contributor

I have remover doit from other return calls. I will also look for better places where doit() could be called.

Contributor

shiprabanga commented Mar 25, 2014

I have remover doit from other return calls. I will also look for better places where doit() could be called.

@hargup

This comment has been minimized.

Show comment Hide comment
@hargup

hargup Apr 23, 2014

Member

@shiprabanga @asmeurer What is the status of this PR?

Member

hargup commented Apr 23, 2014

@shiprabanga @asmeurer What is the status of this PR?

@smichr

This comment has been minimized.

Show comment Hide comment
@smichr

smichr Apr 23, 2014

Member

👎 doit can be quite expensive and should be left to the user's discretion. It is possible to simplify an expression in terms of unevaluated objects. Should simplify get the keyword doit to indicate that doit should be used? No. The user can just...do it: simplify(foo.doit()) instead of simplify(foo, doit=True)

Member

smichr commented Apr 23, 2014

👎 doit can be quite expensive and should be left to the user's discretion. It is possible to simplify an expression in terms of unevaluated objects. Should simplify get the keyword doit to indicate that doit should be used? No. The user can just...do it: simplify(foo.doit()) instead of simplify(foo, doit=True)

@asmeurer

This comment has been minimized.

Show comment Hide comment
@asmeurer

asmeurer Apr 29, 2014

Owner

Well doit is a simplification, and most people who are new to SymPy will expect simplify to do what doit does.

Owner

asmeurer commented Apr 29, 2014

Well doit is a simplification, and most people who are new to SymPy will expect simplify to do what doit does.

@smichr

This comment has been minimized.

Show comment Hide comment
@smichr

smichr Sep 5, 2014

Member

Well doit is a simplification, and most people who are new to SymPy will expect simplify to do what doit does.

The concept of doit and simplify are very distinct. doit implies some action which must be taken to change something unevaluated into something evaluated (Integral -> expr). Using doit will not guarantee that the expression is simplified, just that it is evaluated as far as possible. I could be pursuaded, but my gut feeling is that a note should be put in simplify's docstring saying that doit is not the same as simplify and the user should quickly learn the difference. I could be in favor of a doit flag (default=True) that could be set to False if so desired. e.g. if I were solving an equation I might want to simplify the expression but not apply doit to things that don't depend on the symbol of interest.

Member

smichr commented Sep 5, 2014

Well doit is a simplification, and most people who are new to SymPy will expect simplify to do what doit does.

The concept of doit and simplify are very distinct. doit implies some action which must be taken to change something unevaluated into something evaluated (Integral -> expr). Using doit will not guarantee that the expression is simplified, just that it is evaluated as far as possible. I could be pursuaded, but my gut feeling is that a note should be put in simplify's docstring saying that doit is not the same as simplify and the user should quickly learn the difference. I could be in favor of a doit flag (default=True) that could be set to False if so desired. e.g. if I were solving an equation I might want to simplify the expression but not apply doit to things that don't depend on the symbol of interest.

@asmeurer

This comment has been minimized.

Show comment Hide comment
@asmeurer

asmeurer Sep 5, 2014

Owner

But the fact is, people expect simplify to do what doit does.

Owner

asmeurer commented Sep 5, 2014

But the fact is, people expect simplify to do what doit does.

@asmeurer

This comment has been minimized.

Show comment Hide comment
@asmeurer

asmeurer Sep 5, 2014

Owner

Adding a flag seems fine to me.

Owner

asmeurer commented Sep 5, 2014

Adding a flag seems fine to me.

@comer

This comment has been minimized.

Show comment Hide comment
@comer

comer Sep 6, 2014

Contributor

I agree, being such a user most often!

On Friday, September 5, 2014, Aaron Meurer notifications@github.com wrote:

But the fact is, people expect simplify to do what doit does.


Reply to this email directly or view it on GitHub
#2936 (comment).

Contributor

comer commented Sep 6, 2014

I agree, being such a user most often!

On Friday, September 5, 2014, Aaron Meurer notifications@github.com wrote:

But the fact is, people expect simplify to do what doit does.


Reply to this email directly or view it on GitHub
#2936 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment