Opened 5 years ago

Closed 5 years ago

Last modified 4 years ago

#22120 closed defect (fixed)

powers of 1 in QQbar

Reported by: dkrenn Owned by:
Priority: major Milestone: sage-7.5
Component: algebra Keywords:
Cc: cheuberg Merged in:
Authors: Daniel Krenn Reviewers: Clemens Heuberger
Report Upstream: N/A Work issues:
Branch: 064f552 (Commits, GitHub, GitLab) Commit:
Dependencies: Stopgaps:

Status badges

Description

The following does not work at the moment:

QQbar(1)^QQbar(sqrt(2))
QQ(1)^QQbar(sqrt(3))
ZZ(1)^QQbar(sqrt(5))

Change History (21)

comment:1 Changed 5 years ago by dkrenn

  • Branch set to u/dkrenn/one-qqbar

comment:2 Changed 5 years ago by dkrenn

  • Authors set to Daniel Krenn
  • Commit set to 60abddb6180598f62bf2384b258e26e3d91707ba
  • Status changed from new to needs_review

Patchbot...please test ;)


New commits:

7d58425Trac #22120: make 1^QQbar_element work
60abddbTrac #22120: additional doctest for asymptotic rings

comment:3 Changed 5 years ago by git

  • Commit changed from 60abddb6180598f62bf2384b258e26e3d91707ba to 1eba0e200088b0422579caa740119125012282e6

Branch pushed to git repo; I updated commit sha1. New commits:

b401309Trac #22120: make 1^QQbar_element work
1eba0e2Trac #22120: additional doctest for asymptotic rings

comment:4 Changed 5 years ago by cheuberg

  • Branch changed from u/dkrenn/one-qqbar to u/cheuberg/one-qqbar

comment:5 Changed 5 years ago by git

  • Commit changed from 1eba0e200088b0422579caa740119125012282e6 to 064f5525ddfddbdd61611e6d3b604cf41bc54424

Branch pushed to git repo; I updated commit sha1. New commits:

064f552Trac #22120: mention trac ticket in test

comment:6 follow-up: Changed 5 years ago by cheuberg

  • Reviewers set to Clemens Heuberger

LGTM. Please cross-review my changes and set to positive_review provided that a patchbot agrees.

comment:7 in reply to: ↑ 6 Changed 5 years ago by dkrenn

  • Status changed from needs_review to positive_review

Replying to cheuberg:

LGTM. Please cross-review my changes and set to positive_review provided that a patchbot agrees.

Thank you for your changes. Patchbot agrees as well.

comment:8 Changed 5 years ago by vbraun

  • Branch changed from u/cheuberg/one-qqbar to 064f5525ddfddbdd61611e6d3b604cf41bc54424
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:9 follow-up: Changed 5 years ago by vdelecroix

  • Commit 064f5525ddfddbdd61611e6d3b604cf41bc54424 deleted

Hello,

What was the rationale for this ticket? I have two objections to it

  • In Sage we try to make any binary operation op(x, y) works independently of the values of x and y but only on their parents. This is the base of the whole coercion model. Having pow(1, x) works inconditionally is misleading (there are terrible counterexample to my assertion).
  • More importantly, the test my_qqbar_number == QQbar.one() can be very costly. You are possibly forcing an exactification in QQbar just to allow a very useless corner case. On sage-7.5.1 we had
    sage: s = sum(AA(i)**(1/i) for i in range(1,20))
    sage: %timeit (s/s) ** 4
    10000 loops, best of 3: 55 µs per loop
    
    but with your patch
    sage: s = sum(AA(i)**(1/i) for i in range(1,20))
    sage: (s/s) ** 4  # you can wait ...
    

Looking forward to your answer.

Vincent

comment:10 in reply to: ↑ 9 ; follow-ups: Changed 5 years ago by dkrenn

Hello Vincent,

Replying to vdelecroix:

What was the rationale for this ticket?

1^my_qqbar_number is mathematically meaningful.

  • In Sage we try to make any binary operation op(x, y) works independently of the values of x and y but only on their parents. This is the base of the whole coercion model. Having pow(1, x) works inconditionally is misleading (there are terrible counterexample to my assertion).

I completely agree that the parent of the output should only be influenced by the parents of the inputs and not its particular values. And, after the patch of this ticket, pow of a QQbar element to a QQbar element is (still) always a QQbar element (unless it is not possible and an error is thrown).

FWIW, as it is implemented now, pow and coercions unfortunately do not play well together (only basic stuff, like if there is a direct coercion (i.e. no new parent has to be found), works)

  • More importantly, the test my_qqbar_number == QQbar.one() can be very costly. You are possibly forcing an exactification in QQbar just to allow a very useless corner case. On sage-7.5.1 we had
    sage: s = sum(AA(i)**(1/i) for i in range(1,20))
    sage: %timeit (s/s) ** 4
    10000 loops, best of 3: 55 µs per loop
    
    but with your patch
    sage: s = sum(AA(i)**(1/i) for i in range(1,20))
    sage: (s/s) ** 4  # you can wait ...
    

Oh, indeed, this is a problem and need to be changed.

just to allow a very useless corner case

I disagree with your "useless". As mentioned, 1 to some QQbar element is mathematically meaningful, and some people care about being able to calculate things.

What about checking if the element is "trivially" 1 (in the same spirit as the symbolic expression's is_trivial_zero)? Thus no exactification would be done, but s=QQbar(1) would still be recognized.

comment:11 in reply to: ↑ 10 Changed 5 years ago by vdelecroix

Replying to dkrenn:

Replying to vdelecroix: What about checking if the element is "trivially" 1 (in the same spirit as the symbolic expression's is_trivial_zero)? Thus no exactification would be done, but s=QQbar(1) would still be recognized.

Would already be better...

from sage.rings.qqbar import ANRational

def is_trivially_one(x):
    descr = x._descr
    return isinstance(descr, ANRational) and descr._value.is_one()
Last edited 5 years ago by vdelecroix (previous) (diff)

comment:12 Changed 5 years ago by dkrenn

Follow-up ticket #22250 created.

comment:13 in reply to: ↑ 10 ; follow-up: Changed 5 years ago by dkrenn

Replying to dkrenn:

On sage-7.5.1 we had

sage: s = sum(AA(i)**(1/i) for i in range(1,20))
sage: %timeit (s/s) ** 4
10000 loops, best of 3: 55 µs per loop

I am not sure, what I am doing wrong, but on my 7.5 this already takes ages...

comment:14 in reply to: ↑ 13 ; follow-up: Changed 5 years ago by vdelecroix

Replying to dkrenn:

Replying to dkrenn:

On sage-7.5.1 we had

sage: s = sum(AA(i)**(1/i) for i in range(1,20))
sage: %timeit (s/s) ** 4
10000 loops, best of 3: 55 µs per loop

I am not sure, what I am doing wrong, but on my 7.5 this already takes ages...

Which step?

comment:15 in reply to: ↑ 14 ; follow-up: Changed 5 years ago by dkrenn

Replying to vdelecroix:

Replying to dkrenn:

Replying to dkrenn:

On sage-7.5.1 we had

sage: s = sum(AA(i)**(1/i) for i in range(1,20))
sage: %timeit (s/s) ** 4
10000 loops, best of 3: 55 µs per loop

I am not sure, what I am doing wrong, but on my 7.5 this already takes ages...

Which step?

Let me reformulate:

sage: s = sum(AA(i)**(1/i) for i in range(1,20))
sage: (s/s) ** 4

does not give a result within a couple of hours on a fresh 7.5. And I was wondering why?

comment:16 follow-ups: Changed 5 years ago by vdelecroix

What about the traceback after Ctrl-C?

random guess: it might be relative to _repr_. You can try a = (s/s)**4 instead.

comment:17 in reply to: ↑ 16 Changed 5 years ago by dkrenn

Replying to vdelecroix:

What about the traceback after Ctrl-C?

sage: s = sum(AA(i)**(1/i) for i in range(1,20))
sage: a = (s/s)**4
^C---------------------------------------------------------------------------
KeyboardInterrupt                         Traceback (most recent call last)
<ipython-input-2-00135d41d691> in <module>()
----> 1 a = (s/s)**Integer(4)

/local/dakrenn/sage/7.5/local/lib/python2.7/site-packages/sage/rings/qqbar.pyc in __pow__(self, e)
   4465         d = e.denominator()
   4466         if d == 1:
-> 4467             return generic_power(self, n)
   4468 
   4469         # First, check for exact roots.

/local/dakrenn/sage/7.5/src/sage/structure/element.pyx in sage.structure.element.generic_power (/local/dakrenn/sage/7.5/src/build/cythonized/sage/structure/element.c:27899)()
   4111     """
   4112 
-> 4113     return generic_power_c(a,n,one)
   4114 
   4115 cdef generic_power_c(a, nn, one):

/local/dakrenn/sage/7.5/src/sage/structure/element.pyx in sage.structure.element.generic_power_c (/local/dakrenn/sage/7.5/src/build/cythonized/sage/structure/element.c:28765)()
   4159     # check for idempotence, and store the result otherwise
   4160     aa = a*a
-> 4161     if aa == a:
   4162         return a
   4163 

/local/dakrenn/sage/7.5/src/sage/structure/element.pyx in sage.structure.element.Element.__richcmp__ (/local/dakrenn/sage/7.5/src/build/cythonized/sage/structure/element.c:10359)()
   1121             # an instance of Element. The explicit casts below make
   1122             # Cython generate optimized code for this call.
-> 1123             return (<Element>self)._richcmp_(other, op)
   1124         else:
   1125             return coercion_model.richcmp(self, other, op)

/local/dakrenn/sage/7.5/src/sage/structure/element.pyx in sage.structure.element.Element._richcmp_ (/local/dakrenn/sage/7.5/src/build/cythonized/sage/structure/element.c:10558)()
   1156         cdef int c
   1157         try:
-> 1158             c = left._cmp_(right)
   1159         except NotImplementedError:
   1160             # Check equality by id(), knowing that left is not right

/local/dakrenn/sage/7.5/src/sage/structure/element.pyx in sage.structure.element.Element._cmp_ (/local/dakrenn/sage/7.5/src/build/cythonized/sage/structure/element.c:10947)()
   1172         left_cmp = left.__cmp__
   1173         if isinstance(left_cmp, MethodType):
-> 1174             return left_cmp(right)
   1175         msg = LazyFormat("comparison not implemented for %r")%type(left)
   1176         raise NotImplementedError(msg)

/local/dakrenn/sage/7.5/local/lib/python2.7/site-packages/sage/rings/qqbar.pyc in __cmp__(self, other)
   4405             return -other.sign()
   4406         else:
-> 4407             return self._sub_(other).sign()
   4408 
   4409     def __pow__(self, e):

/local/dakrenn/sage/7.5/local/lib/python2.7/site-packages/sage/rings/qqbar.pyc in sign(self)
   4734             # OK, we'll try adding precision one more time
   4735             self._more_precision()
-> 4736             return self.sign()
   4737         else:
   4738             # Sigh...

/local/dakrenn/sage/7.5/local/lib/python2.7/site-packages/sage/rings/qqbar.pyc in sign(self)
   4737         else:
   4738             # Sigh...
-> 4739             self.exactify()
   4740             return self.sign()
   4741 

/local/dakrenn/sage/7.5/local/lib/python2.7/site-packages/sage/rings/qqbar.pyc in exactify(self)
   3269         od = self._descr
   3270         if isinstance(od, (ANRational, ANExtensionElement)): return
-> 3271         self._set_descr(self._descr.exactify())
   3272 
   3273     def _set_descr(self, new_descr):

/local/dakrenn/sage/7.5/local/lib/python2.7/site-packages/sage/rings/qqbar.pyc in exactify(self)
   7074             left = self._left
   7075             right = self._right
-> 7076             left.exactify()
   7077             right.exactify()
   7078             gen = left._exact_field().union(right._exact_field())

/local/dakrenn/sage/7.5/local/lib/python2.7/site-packages/sage/rings/qqbar.pyc in exactify(self)
   3269         od = self._descr
   3270         if isinstance(od, (ANRational, ANExtensionElement)): return
-> 3271         self._set_descr(self._descr.exactify())
   3272 
   3273     def _set_descr(self, new_descr):

/local/dakrenn/sage/7.5/local/lib/python2.7/site-packages/sage/rings/qqbar.pyc in exactify(self)
   7074             left = self._left
   7075             right = self._right
-> 7076             left.exactify()
   7077             right.exactify()
   7078             gen = left._exact_field().union(right._exact_field())

/local/dakrenn/sage/7.5/local/lib/python2.7/site-packages/sage/rings/qqbar.pyc in exactify(self)
   3269         od = self._descr
   3270         if isinstance(od, (ANRational, ANExtensionElement)): return
-> 3271         self._set_descr(self._descr.exactify())
   3272 
   3273     def _set_descr(self, new_descr):

/local/dakrenn/sage/7.5/local/lib/python2.7/site-packages/sage/rings/qqbar.pyc in exactify(self)
   7074             left = self._left
   7075             right = self._right
-> 7076             left.exactify()
   7077             right.exactify()
   7078             gen = left._exact_field().union(right._exact_field())

/local/dakrenn/sage/7.5/local/lib/python2.7/site-packages/sage/rings/qqbar.pyc in exactify(self)
   3269         od = self._descr
   3270         if isinstance(od, (ANRational, ANExtensionElement)): return
-> 3271         self._set_descr(self._descr.exactify())
   3272 
   3273     def _set_descr(self, new_descr):

/local/dakrenn/sage/7.5/local/lib/python2.7/site-packages/sage/rings/qqbar.pyc in exactify(self)
   7074             left = self._left
   7075             right = self._right
-> 7076             left.exactify()
   7077             right.exactify()
   7078             gen = left._exact_field().union(right._exact_field())

/local/dakrenn/sage/7.5/local/lib/python2.7/site-packages/sage/rings/qqbar.pyc in exactify(self)
   3269         od = self._descr
   3270         if isinstance(od, (ANRational, ANExtensionElement)): return
-> 3271         self._set_descr(self._descr.exactify())
   3272 
   3273     def _set_descr(self, new_descr):

/local/dakrenn/sage/7.5/local/lib/python2.7/site-packages/sage/rings/qqbar.pyc in exactify(self)
   7074             left = self._left
   7075             right = self._right
-> 7076             left.exactify()
   7077             right.exactify()
   7078             gen = left._exact_field().union(right._exact_field())

/local/dakrenn/sage/7.5/local/lib/python2.7/site-packages/sage/rings/qqbar.pyc in exactify(self)
   3269         od = self._descr
   3270         if isinstance(od, (ANRational, ANExtensionElement)): return
-> 3271         self._set_descr(self._descr.exactify())
   3272 
   3273     def _set_descr(self, new_descr):

/local/dakrenn/sage/7.5/local/lib/python2.7/site-packages/sage/rings/qqbar.pyc in exactify(self)
   7074             left = self._left
   7075             right = self._right
-> 7076             left.exactify()
   7077             right.exactify()
   7078             gen = left._exact_field().union(right._exact_field())

/local/dakrenn/sage/7.5/local/lib/python2.7/site-packages/sage/rings/qqbar.pyc in exactify(self)
   3269         od = self._descr
   3270         if isinstance(od, (ANRational, ANExtensionElement)): return
-> 3271         self._set_descr(self._descr.exactify())
   3272 
   3273     def _set_descr(self, new_descr):

/local/dakrenn/sage/7.5/local/lib/python2.7/site-packages/sage/rings/qqbar.pyc in exactify(self)
   7074             left = self._left
   7075             right = self._right
-> 7076             left.exactify()
   7077             right.exactify()
   7078             gen = left._exact_field().union(right._exact_field())

/local/dakrenn/sage/7.5/local/lib/python2.7/site-packages/sage/rings/qqbar.pyc in exactify(self)
   3269         od = self._descr
   3270         if isinstance(od, (ANRational, ANExtensionElement)): return
-> 3271         self._set_descr(self._descr.exactify())
   3272 
   3273     def _set_descr(self, new_descr):

/local/dakrenn/sage/7.5/local/lib/python2.7/site-packages/sage/rings/qqbar.pyc in exactify(self)
   7074             left = self._left
   7075             right = self._right
-> 7076             left.exactify()
   7077             right.exactify()
   7078             gen = left._exact_field().union(right._exact_field())

/local/dakrenn/sage/7.5/local/lib/python2.7/site-packages/sage/rings/qqbar.pyc in exactify(self)
   3269         od = self._descr
   3270         if isinstance(od, (ANRational, ANExtensionElement)): return
-> 3271         self._set_descr(self._descr.exactify())
   3272 
   3273     def _set_descr(self, new_descr):

/local/dakrenn/sage/7.5/local/lib/python2.7/site-packages/sage/rings/qqbar.pyc in exactify(self)
   7074             left = self._left
   7075             right = self._right
-> 7076             left.exactify()
   7077             right.exactify()
   7078             gen = left._exact_field().union(right._exact_field())

/local/dakrenn/sage/7.5/local/lib/python2.7/site-packages/sage/rings/qqbar.pyc in exactify(self)
   3269         od = self._descr
   3270         if isinstance(od, (ANRational, ANExtensionElement)): return
-> 3271         self._set_descr(self._descr.exactify())
   3272 
   3273     def _set_descr(self, new_descr):

/local/dakrenn/sage/7.5/local/lib/python2.7/site-packages/sage/rings/qqbar.pyc in exactify(self)
   7074             left = self._left
   7075             right = self._right
-> 7076             left.exactify()
   7077             right.exactify()
   7078             gen = left._exact_field().union(right._exact_field())

/local/dakrenn/sage/7.5/local/lib/python2.7/site-packages/sage/rings/qqbar.pyc in exactify(self)
   3269         od = self._descr
   3270         if isinstance(od, (ANRational, ANExtensionElement)): return
-> 3271         self._set_descr(self._descr.exactify())
   3272 
   3273     def _set_descr(self, new_descr):

/local/dakrenn/sage/7.5/local/lib/python2.7/site-packages/sage/rings/qqbar.pyc in exactify(self)
   7074             left = self._left
   7075             right = self._right
-> 7076             left.exactify()
   7077             right.exactify()
   7078             gen = left._exact_field().union(right._exact_field())

/local/dakrenn/sage/7.5/local/lib/python2.7/site-packages/sage/rings/qqbar.pyc in exactify(self)
   3269         od = self._descr
   3270         if isinstance(od, (ANRational, ANExtensionElement)): return
-> 3271         self._set_descr(self._descr.exactify())
   3272 
   3273     def _set_descr(self, new_descr):

/local/dakrenn/sage/7.5/local/lib/python2.7/site-packages/sage/rings/qqbar.pyc in exactify(self)
   7074             left = self._left
   7075             right = self._right
-> 7076             left.exactify()
   7077             right.exactify()
   7078             gen = left._exact_field().union(right._exact_field())

/local/dakrenn/sage/7.5/local/lib/python2.7/site-packages/sage/rings/qqbar.pyc in exactify(self)
   3269         od = self._descr
   3270         if isinstance(od, (ANRational, ANExtensionElement)): return
-> 3271         self._set_descr(self._descr.exactify())
   3272 
   3273     def _set_descr(self, new_descr):

/local/dakrenn/sage/7.5/local/lib/python2.7/site-packages/sage/rings/qqbar.pyc in exactify(self)
   7074             left = self._left
   7075             right = self._right
-> 7076             left.exactify()
   7077             right.exactify()
   7078             gen = left._exact_field().union(right._exact_field())

/local/dakrenn/sage/7.5/local/lib/python2.7/site-packages/sage/rings/qqbar.pyc in exactify(self)
   3269         od = self._descr
   3270         if isinstance(od, (ANRational, ANExtensionElement)): return
-> 3271         self._set_descr(self._descr.exactify())
   3272 
   3273     def _set_descr(self, new_descr):

/local/dakrenn/sage/7.5/local/lib/python2.7/site-packages/sage/rings/qqbar.pyc in exactify(self)
   7074             left = self._left
   7075             right = self._right
-> 7076             left.exactify()
   7077             right.exactify()
   7078             gen = left._exact_field().union(right._exact_field())

/local/dakrenn/sage/7.5/local/lib/python2.7/site-packages/sage/rings/qqbar.pyc in exactify(self)
   3269         od = self._descr
   3270         if isinstance(od, (ANRational, ANExtensionElement)): return
-> 3271         self._set_descr(self._descr.exactify())
   3272 
   3273     def _set_descr(self, new_descr):

/local/dakrenn/sage/7.5/local/lib/python2.7/site-packages/sage/rings/qqbar.pyc in exactify(self)
   7074             left = self._left
   7075             right = self._right
-> 7076             left.exactify()
   7077             right.exactify()
   7078             gen = left._exact_field().union(right._exact_field())

/local/dakrenn/sage/7.5/local/lib/python2.7/site-packages/sage/rings/qqbar.pyc in exactify(self)
   3269         od = self._descr
   3270         if isinstance(od, (ANRational, ANExtensionElement)): return
-> 3271         self._set_descr(self._descr.exactify())
   3272 
   3273     def _set_descr(self, new_descr):

/local/dakrenn/sage/7.5/local/lib/python2.7/site-packages/sage/rings/qqbar.pyc in exactify(self)
   7074             left = self._left
   7075             right = self._right
-> 7076             left.exactify()
   7077             right.exactify()
   7078             gen = left._exact_field().union(right._exact_field())

/local/dakrenn/sage/7.5/local/lib/python2.7/site-packages/sage/rings/qqbar.pyc in exactify(self)
   3269         od = self._descr
   3270         if isinstance(od, (ANRational, ANExtensionElement)): return
-> 3271         self._set_descr(self._descr.exactify())
   3272 
   3273     def _set_descr(self, new_descr):

/local/dakrenn/sage/7.5/local/lib/python2.7/site-packages/sage/rings/qqbar.pyc in exactify(self)
   7076             left.exactify()
   7077             right.exactify()
-> 7078             gen = left._exact_field().union(right._exact_field())
   7079             left_value = gen(left._exact_value())
   7080             right_value = gen(right._exact_value())

/local/dakrenn/sage/7.5/local/lib/python2.7/site-packages/sage/rings/qqbar.pyc in union(self, other)
   2408         newpol_sage_y = QQy(newpol_sage)
   2409 
-> 2410         red_elt, red_back, red_pol = do_polred(newpol_sage_y)
   2411 
   2412         red_back_x = QQx(red_back)

/local/dakrenn/sage/7.5/local/lib/python2.7/site-packages/sage/rings/qqbar.pyc in do_polred(poly)
   1711         (1/4*x, 4*x, x^4 - 268435456*x^2 + 211973662764908353025)
   1712     """
-> 1713     new_poly, elt_back = poly._pari_().polredbest(flag=1)
   1714 
   1715     parent = poly.parent()

/local/dakrenn/sage/7.5/local/lib/python2.7/site-packages/sage/libs/cypari2/auto_gen.pxi in sage.libs.cypari2.gen.gen_auto.polredbest (/local/dakrenn/sage/7.5/src/build/cythonized/sage/libs/cypari2/gen.c:80864)()
  15908         """
  15909         cdef GEN _T = T.g
> 15910         sig_on()
  15911         cdef GEN _ret = polredbest(_T, flag)
  15912         return new_gen(_ret)

src/cysignals/signals.pyx in cysignals.signals.sig_raise_exception (build/src/cysignals/signals.c:1303)()

KeyboardInterrupt: 

comment:18 in reply to: ↑ 16 Changed 5 years ago by dkrenn

Replying to vdelecroix:

random guess: it might be relative to _repr_. You can try a = (s/s)**4 instead.

Does not seem to change anything.

comment:19 in reply to: ↑ 15 Changed 5 years ago by dkrenn

Dear Vincent,

Replying to dkrenn:

Let me reformulate:

sage: s = sum(AA(i)**(1/i) for i in range(1,20))
sage: (s/s) ** 4

does not give a result within a couple of hours on a fresh 7.5. And I was wondering why?

I've tried to investigate this now: On all systems and Sage-versions (with and without this patch) I've tested it, this takes ages. I have no idea how it could be that fast on your system. Can you maybe repeat the test or do you have any (other) guess what happens?

(The problem is, that the generic pow tests for idempotence a**2 == a, which is not possible for our example above.)

comment:20 Changed 4 years ago by jdemeyer

Why special case powers of 1 in QQbar? This makes no sense since we don't support QQbar^QQbar powering in general. I'd prefer to revert this in #5574 unless somebody gives me a good reason for why this ticket sense.

comment:21 Changed 4 years ago by jdemeyer

Actually, I created #24490 to revert this.

Note: See TracTickets for help on using tickets.