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

converting from symbolic ring to int is broken #9627

Closed
syazdani77 mannequin opened this issue Jul 28, 2010 · 6 comments
Closed

converting from symbolic ring to int is broken #9627

syazdani77 mannequin opened this issue Jul 28, 2010 · 6 comments

Comments

@syazdani77
Copy link
Mannequin

syazdani77 mannequin commented Jul 28, 2010

Here is simple example:

sage: h = 3^64;
sage: int(h)==int(SR(h))
FALSE

Looking a bit deeper into this, it seems that the first 100 bits are correct, and after that int(SR(h)) is just zeroes. (As a side note, the conversion to ZZ works without a problem.)

CC: @katestange @orlitzky

Component: symbolics

Reviewer: Burcin Erocal

Issue created by migration from https://trac.sagemath.org/ticket/9627

@syazdani77 syazdani77 mannequin added c: coercion labels Jul 28, 2010
@syazdani77 syazdani77 mannequin assigned robertwb Jul 28, 2010
@tornaria
Copy link
Contributor

comment:1

Yikes!

    def __int__(self):
        """
        EXAMPLES::
        
            sage: int(sin(2)*100)
            90
            sage: int(log(8)/log(2))
            3
        """
        #FIXME: can we do better?
        return int(self.n(prec=100))

Can we just change that to

        return int(self._integer_())

?

@tornaria
Copy link
Contributor

comment:2

OTOH,

sage: ZZ(log(8)/log(2))
...
TypeError: unable to convert x (=log(8)/log(2)) to an integer

but

sage: int(log(8)/log(2))  
3

OTOH, int(sin(2)*100) is not equal to 90... not an integer anyway... What's the expected meaning of int(x)?

@sagetrac-cwitty
Copy link
Mannequin

sagetrac-cwitty mannequin commented Aug 3, 2010

comment:3

Pure Python implements int() of a float as truncating toward zero:

>>> int(3.14159)
3
>>> int(-3.14159)
-3
>>> int(2.71828)
2
>>> int(-2.71828)
-2

I think in general we've implemented int() on real numbers of various types as truncating toward zero to follow this precedent.

@tornaria
Copy link
Contributor

tornaria commented Aug 3, 2010

comment:4

Something like this may work:

def SR_int(x):
    """
    sage: SR_int(sin(2)*100)
    90
    sage: SR_int(-sin(2)*100)
    -90
    sage: SR_int(log(8)/log(2))
    3
    sage: SR_int(-log(8)/log(2))
    -3
    sage: SR_int(SR(3^64)) == 3^64
    True
    sage: SR_int(SR(10^100)) == 10^100
    True
    sage: SR_int(SR(10^100+10^-100)) == 10^100
    True
    sage: SR_int(SR(10^100-10^-100)) == 10^100 - 1
    True

    sage: SR_int(sqrt(-1))
    ...
    ValueError: math domain error
    sage: SR_int(x)
    ...
    ValueError: math domain error
    """
    try:
        if x in RR:
            ## truncate toward zero
            y = floor(abs(x))
            if parent(y) is ZZ:
                return int(ZZ(sign(x) * y))
    except:
        pass
    raise ValueError, "math domain error"

Note that floor(log(8)/log(2)) takes about 1s to complete, which means SR_int(log(8)/log(2)) is waaay slower than int(log(8)/log(2). But this is at the cost of int(x) being incorrect when x is very close to an integer (cf. int(SR(10<sup>20-10</sup>-20))).

OTOH, (log(8)/log(2)).full_simplify() takes 35ms to give 3, so it may be worth revisiting the floor() strategy. Opens a can of worms, I guess...


I won't turn the snippet above into a patch, if somebody likes it and wants to produce a patch, go ahead.

@burcin
Copy link

burcin commented May 22, 2012

Reviewer: Burcin Erocal

@burcin
Copy link

burcin commented May 22, 2012

comment:7

#12968 has a patch with a positive review which fixes this. We should close this as a duplicate.

@jdemeyer jdemeyer changed the title converting from symbolic ring to int is broken, converting from symbolic ring to int is broken Jun 2, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants