Opened 11 years ago
Closed 9 years ago
#9627 closed defect (duplicate)
converting from symbolic ring to int is broken
Reported by: | syazdani | Owned by: | robertwb |
---|---|---|---|
Priority: | major | Milestone: | sage-duplicate/invalid/wontfix |
Component: | symbolics | Keywords: | |
Cc: | katestange, mjo | Merged in: | |
Authors: | Reviewers: | Burcin Erocal | |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
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.)
Change History (9)
comment:1 Changed 11 years ago by
- Status changed from new to needs_work
comment:2 Changed 11 years ago by
- Component changed from coercion to symbolics
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)
?
comment:3 Changed 11 years ago by
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.
comment:4 Changed 11 years ago by
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^20-10^-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.
comment:5 Changed 10 years ago by
- Cc katestange added
comment:6 Changed 9 years ago by
- Cc mjo added
comment:7 Changed 9 years ago by
- Milestone set to sage-duplicate/invalid/wontfix
- Reviewers set to Burcin Erocal
- Status changed from needs_work to positive_review
#12968 has a patch with a positive review which fixes this. We should close this as a duplicate.
comment:8 Changed 9 years ago by
- Summary changed from converting from symbolic ring to int is broken, to converting from symbolic ring to int is broken
comment:9 Changed 9 years ago by
- Resolution set to duplicate
- Status changed from positive_review to closed
Yikes!
Can we just change that to
?