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

Non-informative error raised when computing limit of cos(n*pi) #8157

Open
pelegm opened this Issue Oct 1, 2014 · 5 comments

Comments

Projects
None yet
3 participants
@pelegm
Copy link
Contributor

pelegm commented Oct 1, 2014

I've tried the following:

n = Symbol('n', integer=True)
limit(cos(pi*n), n, oo)

and the error that was raised was:

Traceback (most recent call last):
  File "<ipython-input-46-6030098fccd9>", line 1, in <module>
    limit(cos(pi*n), n, oo)
  File "/usr/local/lib/python2.7/dist-packages/sympy/series/limits.py", line 79, in limit
    r = gruntz(e, z, z0, dir)
  File "/usr/local/lib/python2.7/dist-packages/sympy/series/gruntz.py", line 637, in gruntz
    r = limitinf(e, z)
  File "/usr/local/lib/python2.7/dist-packages/sympy/core/cache.py", line 93, in wrapper
    r = func(*args, **kw_args)
  File "/usr/local/lib/python2.7/dist-packages/sympy/series/gruntz.py", line 420, in limitinf
    c0, e0 = mrv_leadterm(e, x)
  File "/usr/local/lib/python2.7/dist-packages/sympy/core/cache.py", line 93, in wrapper
    r = func(*args, **kw_args)
  File "/usr/local/lib/python2.7/dist-packages/sympy/series/gruntz.py", line 501, in mrv_leadterm
    f, logw = rewrite(exps, Omega, x, w)
  File "/usr/local/lib/python2.7/dist-packages/sympy/series/gruntz.py", line 573, in rewrite
    raise NotImplementedError('Result depends on the sign of %s' % sig)
NotImplementedError: Result depends on the sign of I

which is a weird result. I am not sure what is the sign of I (isn't it I?), but anyway, I thought that this limit should simply be undefined or something similar.

@skirpichev

This comment has been minimized.

Copy link
Contributor

skirpichev commented Oct 1, 2014

I am not sure what is the sign of I (isn't it I?), but anyway,

Yes, it's ImaginaryUnit.

Perhaps, we can check if sig is not a real number. But I don't sure if the gruntz algorithm asserts in this case that the limit doesn't exist.

I thought that this limit should simply be undefined or something similar.

Raise and exception is ok for you? If not, how do you think we should represent "undefined or something similar"?

@pelegm

This comment has been minimized.

Copy link
Contributor

pelegm commented Oct 1, 2014

Raise and exception is ok for you? If not, how do you think we should represent "undefined or something similar"?

That's an interesting question. I think that nan would do, or perhaps undefined constant which is in some sense like nan.

Another, more complex way, would be to return the set of partial limits (in this case, the set {-1, 1}), but that's probably too difficult in the general case.

Finally, if you insist on raising exceptions, something like LimitError which inherits from ArithmeticError might be ok - but in general I think that undefined is a valid result, hence no exception should be raised (this is consistent with S(1)/S(0) not raising an exception).

@skirpichev

This comment has been minimized.

Copy link
Contributor

skirpichev commented Oct 1, 2014

I think that nan

Why do you think so?

Another, more complex way, would be to return the set of partial limits (in this case, the set {-1, 1}), but that's probably too difficult in the general case.

yeah, maybe. see e.g. #5299

something like LimitError which inherits from ArithmeticError might be ok

What will mean this LimitError? And why the hell it should inherits from ArithmeticError? )

this is consistent with S(1)/S(0) not raising an exception

S(1)/S(0) - simple arithmetics. This is exactly the only context where nan is defined and used.

@pelegm

This comment has been minimized.

Copy link
Contributor

pelegm commented Oct 1, 2014

According to @asmeurer, at least as it seems in #5299, it should be nan. I believe that an undefined limit is not a number, but it still is a thing that you can do things with (just like nan). It is by no means an error.

However, if you choose the exception method, it still is possible to do it better than it is today (instead the NotImplementedError). For example, with

class UndefinedLimitError(ArithmeticError):
    pass

or something similar.

@skirpichev

This comment has been minimized.

Copy link
Contributor

skirpichev commented Oct 1, 2014

According to @asmeurer, at least as it seems in #5299, it should be nan.

A reference on unmotivated suggestion still an unmotivated suggestion.

I believe that an undefined limit is not a number, but it still is a thing that you can do things with (just like nan).

You can't "do" anything useful with nan. The nan, roughly, is a "thing" that may appear as a result for some arithmetic expressions.

However, if you choose the exception method, it still is possible to do it better than it is today (instead the NotImplementedError).

Sure. If we know that the limit doesn't exists - we should raise more appropriate exception (however, no idea what ArithmeticError does here). The problem - we don't know.

@jcrist jcrist removed the Priority: Low label Apr 17, 2015

skirpichev added a commit to skirpichev/diofant that referenced this issue Nov 2, 2016

Add regression tests & mention closed issues
    close sympy/sympy#3112 (MrvAsympt was added in diofant#6)
    close sympy/sympy#9173 (test was added in 5a510ac)
    close sympy/sympy#9808 (fixed in 09e539b)
    close sympy/sympy#9341 (fixed in af98a00)
    close sympy/sympy#9908 (fixed in cc3fa8d)
    close sympy/sympy#6171 (test added in d278031)
    close sympy/sympy#9276 (diagnose_imports.py removed in ab8c535)
    close sympy/sympy#10201 (fixed in 0d0fc5f)
    close sympy/sympy#9057 (test was added in 8290a0c)
    close sympy/sympy#11159 (test was added in ffb76cb)
    close sympy/sympy#2839 (new AST transformers are used, see diofant#278 and diofant#167)
    close sympy/sympy#11081 (see ed01e16 and bb92329)
    close sympy/sympy#10974 (see 73fc425)
    close sympy/sympy#10806 (test in 539929a)
    close sympy/sympy#10801 (test in 2fe3da5)
    close sympy/sympy#9549 (test in 88bdefa)
    close sympy/sympy#4231 (test was added in fb411d5)
    close sympy/sympy#8634 (see 2fcbb58)
    close sympy/sympy#8481 (see 1ef20d3)
    close sympy/sympy#9956 (fixed in a34735f)
    close sympy/sympy#9747 (see e117c60)
    close sympy/sympy#7853 (see 3e4fbed)
    close sympy/sympy#9634 (see 2be03f5)
    close sympy/sympy#8500 (fixed in diofant#104 and finally in diofant#316)
    close sympy/sympy#9192 (see 9bf622f)
    close sympy/sympy#7130 (see e068fa3)
    close sympy/sympy#8514 (see b2d543b)
    close sympy/sympy#9334 (see 90de625)
    close sympy/sympy#8229 (see 9755b89)
    close sympy/sympy#8061 (see 7054f06)
    close sympy/sympy#7872 (tested in diofant#6)
    close sympy/sympy#3496 (tested in test_log_symbolic)
    close sympy/sympy#2929 (see da7db7a)
    close sympy/sympy#8203 (oo is not a real, see diofant#36)
    close sympy/sympy#7649 (0 is imaginary since diofant#8)
    close sympy/sympy#7256 (fixed in c0a4549)
    close sympy/sympy#6783 (see cb28d63)
    close sympy/sympy#5662 (is_integer issue fixed in 6bfa9f8, there is no is_bounded anymore)
    close sympy/sympy#5295 (fixed with diofant#354)
    close sympy/sympy#4856 (we now have flake/pep tests)
    close sympy/sympy#4555 (flake8 enabled after diofant#214)
    close sympy/sympy#5773 (cmp_to_key removed after diofant#164 and c9acbf0)
    close sympy/sympy#5484 (see above)

    Added regression tests:
    from https://groups.google.com/forum/#!topic/sympy/LkTMQKC_BOw
    fixes sympy/sympy#8825 (probably via diofant#209)
    fixes sympy/sympy#8635
    fixes sympy/sympy#8157
    fixes sympy/sympy#7872
    fixes sympy/sympy#7599
    fixes sympy/sympy#6179
    fixes sympy/sympy#5415
    fixes sympy/sympy#2865
    fixes sympy/sympy#5907
    fixes sympy/sympy#11722

    Closes diofant#347

skirpichev added a commit to skirpichev/diofant that referenced this issue Nov 2, 2016

Add regression tests & mention closed issues
    close sympy/sympy#3112 (MrvAsympt was added in diofant#6)
    close sympy/sympy#9173 (test was added in 5a510ac)
    close sympy/sympy#9808 (fixed in 09e539b)
    close sympy/sympy#9341 (fixed in af98a00)
    close sympy/sympy#9908 (fixed in cc3fa8d)
    close sympy/sympy#6171 (test added in d278031)
    close sympy/sympy#9276 (diagnose_imports.py removed in ab8c535)
    close sympy/sympy#10201 (fixed in 0d0fc5f)
    close sympy/sympy#9057 (test was added in 8290a0c)
    close sympy/sympy#11159 (test was added in ffb76cb)
    close sympy/sympy#2839 (new AST transformers are used, see diofant#278 and diofant#167)
    close sympy/sympy#11081 (see ed01e16 and bb92329)
    close sympy/sympy#10974 (see 73fc425)
    close sympy/sympy#10806 (test in 539929a)
    close sympy/sympy#10801 (test in 2fe3da5)
    close sympy/sympy#9549 (test in 88bdefa)
    close sympy/sympy#4231 (test was added in fb411d5)
    close sympy/sympy#8634 (see 2fcbb58)
    close sympy/sympy#8481 (see 1ef20d3)
    close sympy/sympy#9956 (fixed in a34735f)
    close sympy/sympy#9747 (see e117c60)
    close sympy/sympy#7853 (see 3e4fbed)
    close sympy/sympy#9634 (see 2be03f5)
    close sympy/sympy#8500 (fixed in diofant#104 and finally in diofant#316)
    close sympy/sympy#9192 (see 9bf622f)
    close sympy/sympy#7130 (see e068fa3)
    close sympy/sympy#8514 (see b2d543b)
    close sympy/sympy#9334 (see 90de625)
    close sympy/sympy#8229 (see 9755b89)
    close sympy/sympy#8061 (see 7054f06)
    close sympy/sympy#7872 (tested in diofant#6)
    close sympy/sympy#3496 (tested in test_log_symbolic)
    close sympy/sympy#2929 (see da7db7a)
    close sympy/sympy#8203 (oo is not a real, see diofant#36)
    close sympy/sympy#7649 (0 is imaginary since diofant#8)
    close sympy/sympy#7256 (fixed in c0a4549)
    close sympy/sympy#6783 (see cb28d63)
    close sympy/sympy#5662 (is_integer issue fixed in 6bfa9f8, there is no is_bounded anymore)
    close sympy/sympy#5295 (fixed with diofant#354)
    close sympy/sympy#4856 (we now have flake/pep tests)
    close sympy/sympy#4555 (flake8 enabled after diofant#214)
    close sympy/sympy#5773 (cmp_to_key removed after diofant#164 and c9acbf0)
    close sympy/sympy#5484 (see above)

    Added regression tests:
    from https://groups.google.com/forum/#!topic/sympy/LkTMQKC_BOw
    fixes sympy/sympy#8825 (probably via diofant#209)
    fixes sympy/sympy#8635
    fixes sympy/sympy#8157
    fixes sympy/sympy#7872
    fixes sympy/sympy#7599
    fixes sympy/sympy#6179
    fixes sympy/sympy#5415
    fixes sympy/sympy#2865
    fixes sympy/sympy#5907
    fixes sympy/sympy#11722

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