sagemath / sage

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

AA.as_number_field_element() sometimes in the wrong ring #35613

Open saraedum opened 1 year ago

saraedum commented 1 year ago

Is there an existing issue for this?

Did you read the documentation and troubleshoot guide?

Environment

- **OS**: conda-forge
- **Sage Version**: SageMath version 9.7, Release Date: 2022-09-19

Steps To Reproduce

sage: AA(1).as_number_field_element()
(Rational Field,
 1,
 Ring morphism:
   From: Rational Field
   To:   Algebraic Real Field
   Defn: 1 |--> 1)
sage: AA(1).as_number_field_element()[1].parent()
Integer Ring

Expected Behavior

I expected the result to live in the Rational Field.

Actual Behavior

The result lives in the Integer Ring.

Additional Information

No response

videlec commented 1 year ago

Seems to come from the fact that ANRational allows Integer and Rational as _value attribute (in qqbar.py)

sage: a = QQbar(1)
sage: a._descr._value
1
sage: type(a._descr._value)
<class 'sage.rings.integer.Integer'>

And this is not taken care of in the following call

sage: type(qq_generator(a._exact_value()))
<class 'sage.rings.integer.Integer'>

Maybe removing lines 3419-20 in sage/rings/qqbar.py would do the job

            sage: gen2_3(sqrt3)
            a^2 - 2
        """
-        if self._trivial:
-            return elt._value
        if isinstance(elt, ANRational):
            return self._field(elt._value)

but the extra type checking might slow down everything, so it might be preferable to do

            sage: gen2_3(sqrt3)
            a^2 - 2
        """
-        if self._trivial:
-            return elt._value
-        if isinstance(elt, ANRational):
+        if self._trivial or isinstance(elt, ANRational):
            return self._field(elt._value)