Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug in caching assumptions #3468

Closed
certik opened this issue Sep 24, 2007 · 11 comments
Closed

bug in caching assumptions #3468

certik opened this issue Sep 24, 2007 · 11 comments

Comments

@certik
Copy link
Member

certik commented Sep 24, 2007

While implementing the issue 3428 , I discovered a very nasty bug. Try this:

In [1]: re(x)
Out[1]: re(x)

In [2]: x.series(x,1)
Out[2]: O(x)

In [3]: re(x)
Out[3]: re(x)

ok, that's fine. Now go to core/function.py and comment out the caching,
like this (line 62):

```
#@cache_it
def __new__(cls, *args, **kwargs):
```

and do it again:

In [1]: re(x)
Out[1]: re(x)

In [2]: x.series(x,1)
Out[2]: O(x)

In [3]: re(x)
Out[3]: x

This bug then influences many other tests etc. Please don't fix it though,
until I commit my changes from the branch.

Original issue for #3468: http://code.google.com/p/sympy/issues/detail?id=369

Original author: https://code.google.com/u/104039945248245758823/

Referenced issues: #3428

@certik
Copy link
Member Author

certik commented Sep 23, 2007

Just for the impatient - the problematic line is the hack in sympy/series/order.py:162:

```
    for s in symbols:
        assume_dict = {}
        if not s.is_infinitesimal:
            assume_dict['infinitesimal'] = True
        if s.is_positive is None:
            assume_dict['positive'] = True
        if assume_dict:
            s.assume(**assume_dict)
```

which adds assumptions to the symbols, and they propagate out of the function, of
course. The caching mechanism probably fixes that.

Note again - please don't fix it, until I commit, probably late at night.

Original comment: http://code.google.com/p/sympy/issues/detail?id=369#c1

Original author: https://code.google.com/u/104039945248245758823/

@certik
Copy link
Member Author

certik commented Sep 29, 2007

Rising this to high, because I need to take some action now, since it is breaking my
new limits code.

**Labels:** -Priority-Medium Priority-High  

Original comment: http://code.google.com/p/sympy/issues/detail?id=369#c2

Original author: https://code.google.com/u/104039945248245758823/

@certik
Copy link
Member Author

certik commented Oct 1, 2007

Lowering to medium, the new limits code seems to work quite well now.

**Summary:** bug in caching assumptions  
**Labels:** -Priority-High Priority-Medium  

Original comment: http://code.google.com/p/sympy/issues/detail?id=369#c3

Original author: https://code.google.com/u/104039945248245758823/

@navytux
Copy link

navytux commented Jan 26, 2008

Smaller steps to reproduce:

In [1]: O(x**2)
Out[1]: O(x**2)

In [2]: re(x)
Out[2]: x

Original comment: http://code.google.com/p/sympy/issues/detail?id=369#c4

Original author: https://code.google.com/u/111152560333599832822/

@navytux
Copy link

navytux commented Jan 26, 2008

Patch sent for review: http://groups.google.com/group/sympy-patches/msg/f0ac17b32be586d8

**Status:** Started  
**Labels:** -Type-Defect Type-Patch  

Original comment: http://code.google.com/p/sympy/issues/detail?id=369#c5

Original author: https://code.google.com/u/111152560333599832822/

@certik
Copy link
Member Author

certik commented Jan 26, 2008

Please commit.

**Labels:** Milestone-Release0.5.12  

Original comment: http://code.google.com/p/sympy/issues/detail?id=369#c6

Original author: https://code.google.com/u/104039945248245758823/

@navytux
Copy link

navytux commented Jan 27, 2008

committed: http://hg.sympy.org/sympy/rev/941cad99f705 What's the state of this issue?

Original comment: http://code.google.com/p/sympy/issues/detail?id=369#c7

Original author: https://code.google.com/u/111152560333599832822/

@certik
Copy link
Member Author

certik commented Jan 27, 2008

I think it's fixed by your patch. Test also looks good. 

Now maybe some other problematic things with series could start working, or at least
be much more easier to fix.

Thanks for the patch!

**Status:** Fixed  

Original comment: http://code.google.com/p/sympy/issues/detail?id=369#c8

Original author: https://code.google.com/u/104039945248245758823/

@navytux
Copy link

navytux commented Jan 27, 2008

Thanks!

I've seen there are some new XFAILs :)
btw: this was so easy to fix, why had we waited so long?

Original comment: http://code.google.com/p/sympy/issues/detail?id=369#c9

Original author: https://code.google.com/u/111152560333599832822/

@certik
Copy link
Member Author

certik commented Jan 27, 2008

> btw: this was so easy to fix, why had we waited so long?

It wasn't easy at all in September. But we polished and fixed sympy a lot since then,
so it was easy now.

Original comment: http://code.google.com/p/sympy/issues/detail?id=369#c10

Original author: https://code.google.com/u/104039945248245758823/

@navytux
Copy link

navytux commented Jan 27, 2008

yep, goodbye old bug.

Original comment: http://code.google.com/p/sympy/issues/detail?id=369#c11

Original author: https://code.google.com/u/111152560333599832822/

@certik certik added this to the Release0.5.12 milestone Mar 7, 2014
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants