sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.31k stars 449 forks source link

something wrong in the initializer for elements of QuaternionAlgebra #5114

Closed ba94b9bb-195b-4422-a5e2-176920eaa163 closed 15 years ago

ba94b9bb-195b-4422-a5e2-176920eaa163 commented 15 years ago

Consider:

sage: QA = QuaternionAlgebra(QQ, -1, -1)
sage: foo = QA(3.0); foo
3.00000000000000
sage: parent(foo)
Quaternion algebra with generators (i, j, k) over Rational Field
sage: parent(foo.vector()[0])
Real Field with 53 bits of precision

I don't think the initializer should let you construct an element with RR members but still claim to be over QQ.

CC: @williamstein

Component: algebra

Author: Alex Ghitza

Reviewer: David Loeffler

Merged: 4.0.1.alpha0

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

aghitza commented 15 years ago
comment:1

I think that QA(3.0) should definitely throw an exception, just as the following does:

sage: K.<a> = QuadraticField(11)
sage: K(3.0)
ERROR: An unexpected error occurred while tokenizing input
The following traceback may be corrupted or invalid
The error message is: ('EOF in multi-line statement', (858, 0))

ERROR: An unexpected error occurred while tokenizing input
The following traceback may be corrupted or invalid
The error message is: ('EOF in multi-line statement', (863, 0))

---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)

/home/ghitza/.sage/temp/artin/9038/_home_ghitza__sage_init_sage_0.py in <module>()

/opt/sage-3.3.alpha0/local/lib/python2.5/site-packages/sage/structure/parent.so in sage.structure.parent.Parent.__call__ (sage/structure/parent.c:3645)()

/opt/sage-3.3.alpha0/local/lib/python2.5/site-packages/sage/structure/coerce_maps.so in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (sage/structure/coerce_maps.c:2793)()

/opt/sage-3.3.alpha0/local/lib/python2.5/site-packages/sage/structure/coerce_maps.so in sage.structure.coerce_maps._call_ (sage/structure/coerce_maps.c:2700)()

/opt/sage-3.3.alpha0/local/lib/python2.5/site-packages/sage/rings/number_field/number_field.pyc in _element_constructor_(self, x)
   1417                 raise ValueError, "vector must be of length equal to the degree of this number field"
   1418             return sum([ x[i]*self.gen(0)**i for i in range(self.degree()) ])
-> 1419         return self._coerce_non_number_field_element_in(x)
   1420 
   1421     def _coerce_from_str(self, x):

/opt/sage-3.3.alpha0/local/lib/python2.5/site-packages/sage/rings/number_field/number_field.pyc in _coerce_non_number_field_element_in(self, x)
   1527         except (TypeError, AttributeError), msg:
   1528             pass
-> 1529         raise TypeError, type(x)
   1530 
   1531     def _coerce_map_from_(self, R):

TypeError: <type 'sage.rings.real_mpfr.RealLiteral'>
aghitza commented 15 years ago
comment:2

For what it's worth, this behaviour changed and became more consistent with the recent reworking of the quaternion algebra code:

----------------------------------------------------------------------
| Sage Version 3.4, Release Date: 2009-03-11                         |
| Type notebook() for the GUI, and license() for information.        |
----------------------------------------------------------------------
sage: sage: QA = QuaternionAlgebra(QQ, -1, -1)
sage: sage: foo = QA(3.0); foo
3
sage: parent(foo)
Quaternion Algebra (-1, -1) with base ring Rational Field
sage: foo[0]
3
sage: parent(foo[0])
Rational Field

Do we want to consider this fixed now?

ba94b9bb-195b-4422-a5e2-176920eaa163 commented 15 years ago
comment:3

Yes, it looks like the bug is fixed; but somebody should create a doctest so that we can make sure it stays fixed forever.

aghitza commented 15 years ago
comment:4

The attached patch adds the requested doctest.

aghitza commented 15 years ago

Attachment: trac_5114.patch.gz

11d1fc49-71a1-44e1-869f-76be013245a0 commented 15 years ago
comment:5

Looks good to me: patch applies fine to 4.0.rc1, all doctests in the quatalg directory pass, and the docs build OK. Positive review.

mwhansen commented 15 years ago
comment:6

Merged in 4.0.1.alpha0.

mwhansen commented 15 years ago

Merged: 4.0.1.alpha0

mwhansen commented 15 years ago

Author: Alex Ghitza

mwhansen commented 15 years ago

Reviewer: David Loeffler