I.parent() should not be the symbolic ring

[videlec]

As suggested in #7545, this ticket defines the imaginary unit I directly as the generator of QuadraticField(-1) instead of wrapping it in a symbolic expression.

Why? To allow it to be used in combination with elements of QQbar, CC, etc., without coercion forcing the expression to SR. For example, 1.0 + I is now an element of CC instead of SR.

How? We set I in sage.all to the generator of ℚ[i], and deprecate importing it from sage.symbolic.I. The symbolic I remains available from sage.symbolic.constants for library code working with symbolic expressions, and as SR(I) or SR.I(). We create a dedicated subclass of quadratic number field elements to make it possible to support features similar to those of symbolic expressions of the form a + I*b that would not make sense for number field elements (or be too hard to implement, or pollute the namespace).

Why not ℤ[i]? Because the class hierarchy of number field and order elements makes it difficult to provide the compatibility features mentioned above for elements of both ℤ[i] and ℚ[i]. Having I be an element of ℚ[i] covers almost all use cases (all except working with algebraic integers?), and people who work with orders are sophisticated enough to explicitly ask for I ∈ ℤ[i] when they need that. (This is a debatable choice. We could probably do without the dedicated subclass for elements of ℚ[i], at the price of breaking backward compatibility a bit more.)

CC: @categorie @jdemeyer @mezzarobba @behackl @rwst @kliem @mwageringel

Component: number fields

Author: Marc Mezzarobba

Branch/Commit: 54a34a7

Reviewer: Vincent Delecroix

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

[nbruin]

comment:1

I'm not so sure it should. Which quadratic field is the appropriate one? There are many, distinguishable by the name of their generator (that would be 'I') for this one, but also by their specified embeddings, and it's not clear which one to choose.

Is there an argument for doing this? Ticket #17860 referenced in the description makes no mention of it. I'd imagine there might be evaluation reasons that might make it attractive. Perhaps those also give an indication of which quadratic field would be the appropriate one.

[videlec]

comment:2

Replying to @nbruin:

I'm not so sure it should. Which quadratic field is the appropriate one? There are many, distinguishable by the name of their generator (that would be 'I') for this one, but also by their specified embeddings, and it's not clear which one to choose.

I also thought of this in the train... and I do not see much possible choices. I found two rather natural choices for the adoption of I:

• the ring of integers Z[sqrt(-1)] with its natural embedding in QQbar
• QQbar itself

Is there an argument for doing this? Ticket #17860 referenced in the description makes no mention of it. I'd imagine there might be evaluation reasons that might make it attractive. Perhaps those also give an indication of which quadratic field would be the appropriate one.

Some reasons (in favor of the first choice):

• I + 1.0 and 1.0 * I should be complex numbers
• factor((I+3)) should be the factorization over the Gaussian integers (i.e. (-I) * (I + 1) * (2*I + 1))
• abs(I) should be the integer 1

Vincent

[jdemeyer]

comment:3

I should be an element of QuadraticField(-1, 'I', embedding=CC.gen(), latex_name='i'), which is what it currently is (see src/sage/symbolic/pynac.pyx).

[jdemeyer]

comment:4

Replying to @nbruin:

Is there an argument for doing this?

In short: the same reason that 1 is not symbolic. When doing basic arithmetic with I, there is no need for a symbolic I. Whenever something symbolic is needed, coercion will make it symbolic.

[pjbruin]

comment:5

Replying to @videlec:

I found two rather natural choices for the adoption of I:

• the ring of integers Z[sqrt(-1)] with its natural embedding in QQbar

The ring ZZ[sqrt(-1)] definitely looks like the most natural choice to me, since admits a canonical homomorphism to any other ring with a distinguished square root of -1.

As for the distinguished embedding, is there a specific reason for choosing QQbar? A more minimal choice would be to fix an embedding into a UniversalCyclotomicField; then we would have coercion maps ZZ[I] -> UniversalCyclotomicField(zeta) -> QQbar -> CC. (Maybe this makes finding common parents slightly harder, though.)

[kcrisman]

comment:6

Thank you all for working on this - this kind of thing has been on the radar for years but after Burcin left day-to-day operations around here there hasn't been the combination of energy and know-how to do this "correctly", whatever that might mean. Just keep in mind it would be nice for I in SR to be true, though I'm sure it will be since 1 in SR already is True. I do like the idea of abs(I) being an Integer and not a symbolic expression.

[mezzarobba]

comment:8

Proof of concept to see what would break: u/mmezzarobba/18036-QQi (I'm using a number field element for now, not an order element, but switching shouldn't be hard). Still needs quite a bit of work, but all tests should pass (I didn't run them all with the last version of the code). Any comment or improvement welcome!

In particular:

• Are there behavior changes that you think are not acceptable, or not acceptable without a deprecation?
• I'm not happy with the changes to sage.geometry.hyperbolic_space (which apparently relied on operations involving I triggering coercions to SR), but I don't understand the code well enough to do better.

Tangentially related: now may be a good time to deprecate (or remove directly?) the bogus coercion from SR to QQbar.

[mezzarobba]

comment:9

As it turns out, there are a few failures in complex_mpc.pyx. But unless I'm mistaken these failures are solved by #14982. And conversely, the present ticket provides a real fix for a problem I only worked around in #14982.

[videlec]

comment:10

Very nice that it worked!

I do not quite understand why you need the creation of a new class of NumberFieldQQi... is that only for the special method you need in the element class?

For embedding in QQbar I guess that what should be fixed is embedding of number fields. In the ideal world, you would declare:

QQi = NumberField(x**2 + 1, 'I', embedding=QQbar.gen())

But then, there might be an infinite loop with the definition of I in QQbar. I had the same sort of troubles when refining default embedding from lazy field to AA/QQbar.

Vincent

[mezzarobba]

comment:11

Replying to @videlec:

I do not quite understand why you need the creation of a new class of NumberFieldQQi... is that only for the special method you need in the element class?

I don't remember, it could be that the reason no longer exists due to later changes.

For embedding in QQbar I guess that what should be fixed is embedding of number fields. In the ideal world, you would declare:

QQi = NumberField(x**2 + 1, 'I', embedding=QQbar.gen())

Yes, the idea is to switch to an embedding into QQbar when other number fields do.

[mezzarobba]

comment:12

Replying to @mezzarobba:

Replying to @videlec:

I do not quite understand why you need the creation of a new class of NumberFieldQQi... is that only for the special method you need in the element class?

I don't remember, it could be that the reason no longer exists due to later changes.

One reason was that having separate classes makes it easy to test if we are in the special case of QQ[i] using isinstance. In the case of the parent class, this is convenient when specifying coercions, for instance.

[videlec]

comment:13

Replying to @mezzarobba:

Replying to @mezzarobba:

Replying to @videlec:

I do not quite understand why you need the creation of a new class of NumberFieldQQi... is that only for the special method you need in the element class?

I don't remember, it could be that the reason no longer exists due to later changes.

One reason was that having separate classes makes it easy to test if we are in the special case of QQ[i] using isinstance. In the case of the parent class, this is convenient when specifying coercions, for instance.

Anyway this will be instantiated at startup so why not keeping one instance QQi in sage.rings.number_field.number_field? (like we have for ZZ, QQ, etc). Then you can test identity when testing coercions.

[mezzarobba]

comment:14

Replying to @videlec:

Anyway this will be instantiated at startup so why not keeping one instance QQi in sage.rings.number_field.number_field? (like we have for ZZ, QQ, etc).

If I remember right, currently, just adding QQi = ...() in the module currently doesn't work due to import order constraints. For now I just kept the creation of QQ[i] happening at the same time as it used to. But that's certainly something we should try to improve after this first draft.

Then you can test identity when testing coercions.

Yes. Having a separate class would also be natural if we want I.parent() to display something less frightening than “Number Field in I with defining polynomial x^2 + 1”, and more generally to implement features specific to QQ[i]. But I can't really think of anything that makes sense for this field and not for embedded quadratic number fields in general, so perhaps it is better to encourage people to always implement a more general version?

A related question is whether QQi is NumberField(x^2+1, 'I', embedding=CC.0) should be true, or if there should be two separate parents.

What do you think?

[videlec]

comment:16

Replying to @mezzarobba:

Replying to @videlec:

Anyway this will be instantiated at startup so why not keeping one instance QQi in sage.rings.number_field.number_field? (like we have for ZZ, QQ, etc).

If I remember right, currently, just adding QQi = ...() in the module currently doesn't work due to import order constraints. For now I just kept the creation of QQ[i] happening at the same time as it used to. But that's certainly something we should try to improve after this first draft.

Here we can probably cheat with

_QQi = None
def NumberFieldQQi():
    if _QQi is None:
        # build it once for all
        ...
    return _QQi

Then you can test identity when testing coercions.

Yes. Having a separate class would also be natural if we want I.parent() to display something less frightening than “Number Field in I with defining polynomial x^2 + 1”, and more generally to implement features specific to QQ[i]. But I can't really think of anything that makes sense for this field and not for embedded quadratic number fields in general, so perhaps it is better to encourage people to always implement a more general version?

Yes! Having a custom representation should be done in the main class. It is already possible:

sage: K = QuadraticField(2)
sage: K.rename('It's me')
sage: K
It's me
sage: K.rename(None)
sage: K
Number Field in a with defining polynomial x^2 - 2

A related question is whether QQi is NumberField(x^2+1, 'I', embedding=CC.0) should be true, or if there should be two separate parents.

What do you think?

More generally, do we want unique representation for (absolute) number fields? I would tend to say yes. And the natural keys would be:

• the polynomial
• the variable name (not of the polynomial!)
• the embedding

Vincent

[mezzarobba]

comment:17

Replying to @videlec:

Yes! Having a custom representation should be done in the main class. It is already possible:

If all we want is a different string representation, yes, perhaps it makes sense to use rename()...

A related question is whether QQi is NumberField(x^2+1, 'I', embedding=CC.0) should be true, or if there should be two separate parents.

What do you think?

More generally, do we want unique representation for (absolute) number fields?

I think everyone agrees that absolute number fields should have unique representation. My question was whether Q[i] should be an absolute number field in this sense, or if it should be a “special” object such that people could work with both Q[i]-as-a-subset-of-complex-numbers and Q[i]-as-a-number field, possibly at the same time. I'd prefer a single object as well, but I am sure I have missed some of the implications, so if anyone has arguments in favor of the other option, I would be interested in hearing them.

[videlec]

comment:18

Replying to @mezzarobba:

A related question is whether QQi is NumberField(x^2+1, 'I', embedding=CC.0) should be true, or if there should be two separate parents.

What do you think?

More generally, do we want unique representation for (absolute) number fields?

I think everyone agrees that absolute number fields should have unique representation. My question was whether Q[i] should be an absolute number field in this sense, or if it should be a “special” object such that people could work with both Q[i]-as-a-subset-of-complex-numbers and Q[i]-as-a-number field, possibly at the same time. I'd prefer a single object as well, but I am sure I have missed some of the implications, so if anyone has arguments in favor of the other option, I would be interested in hearing them.

I would be interested in working with any number field seeing them as a subfield of the real or complex numbers! Not only QQi and it makes sense to ask whether we need a dedicated class for that. For both parent and elements.

Note that it is already partly possible to play with element of number fields as real numbers (especially with quadratic fields)

sage: K.<sqrt2> = QuadraticField(2)
sage: 1 < sqrt2 < 3/2
True
sage: sqrt2.n()
1.41421356237310
sage: sqrt2 + CC(0,1)
1.41421356237310 + 1.00000000000000*I
sage: sage: cos(sqrt2)
cos(sqrt(2))
sage: sqrt2.continued_fraction()
[1; (2)*]

About having methods .cos(), .sin(), .exp(), it is already something which I found dangerous with integers for which the method .sqrt() might return an answer with a different parent

sage: 4.sqrt()  # answer is a Sage integer
2
sage: 2.sqrt()  # answer is symbolic
sqrt(2)

Which is very different from

sage: R.<x> = ZZ[]
sage: ((x+1)**2 * (x-2)**4).sqrt()
x^3 - 3*x^2 + 4
sage: R(2).sqrt()
Traceback (most recent call last):
...
TypeError: Polynomial is not a square. You must specify
the name of the square root when using the default extend = True

At the time Sage would support embedding of number fields into p-adic fields, I think it might be worse to have that dedicated class! But in the meantime, I have no strong opinion.

Vincent

[rwst]

comment:19

See also https://groups.google.com/forum/?hl=en#!topic/sage-devel/Oa0kOC-VGds

[jdemeyer]

comment:20

Replying to @videlec:

About having methods .cos(), .sin(), .exp(), it is already something which I found dangerous with integers

Still, if we ever want this new non-symbolic I to behave like to old symbolic I, we would need to support things like that:

**********************************************************************
File "src/sage/symbolic/expression.pyx", line 7901, in sage.symbolic.expression.Expression.log
Failed example:
    I.log()
Exception raised:
    Traceback (most recent call last):
      File "/usr/local/src/sage-git/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 496, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/usr/local/src/sage-git/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 858, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.symbolic.expression.Expression.log[11]>", line 1, in <module>
        I.log()
      File "sage/structure/element.pyx", line 420, in sage.structure.element.Element.__getattr__ (build/cythonized/sage/structure/element.c:4676)
        return getattr_from_other_class(self, P._abstract_element_class, name)
      File "sage/structure/misc.pyx", line 259, in sage.structure.misc.getattr_from_other_class (build/cythonized/sage/structure/misc.c:1772)
        raise dummy_attribute_error
    AttributeError: 'sage.rings.number_field.number_field_element_quadratic.NumberFieldElement_quadratic' object has no attribute 'log'
**********************************************************************

[jdemeyer]

Branch: u/jdemeyer/i_parent___should_not_be_the_symbolic_ring

[behackl]

Commit: 2f5a519

[behackl]

New commits:

2f5a519

parent(I) should be a number field

[behackl]

comment:24

Hi! I'd like to revive this discussion a bit because we're getting a doctest failure at pynac/pynac#162 which would probably be fixed along the lines of this ticket.

For starters, I had a look at the current branch and some of the resulting doctest failures; this should roughly resemble the tasks that are still to be done:

• sage -t --warn-long 54.2 src/sage/symbolic/constants.py # 3 doctests failed;
  sage -t --warn-long 54.2 src/sage/symbolic/pynac.pyx # 3 doctests failed:

  duplicate doctests. not sure how to fix (maybe switch to I_symbolic?). and which to remove.

• sage -t --warn-long 54.2 src/sage/symbolic/relation.py # 3 doctests failed:

  Doctests can be fixed directly.

• sage -t --warn-long 54.2 src/sage/symbolic/expression_conversions.py # 2 doctests failed

  probably I_symbolic is needed?

• sage -t --warn-long 54.2 src/sage/symbolic/expression.pyx # 14 doctests failed

  • arithmetic with oo is broken (4 doctests)
  • _is_registered_constant_ --> remove doctest? I_symbolic?
  • is_numeric/is_constant missing (I_symbolic?)
  • imag_part/real_part should be an alias of imag/real (3 doctests)
  • I.log not implemented (3 doctests)
  • rectform: either converse to SR or multiply with I_symbolic.

• sage -t --warn-long 54.2 src/sage/rings/infinity.py # 2 doctests failed

  arithmetic with oo is broken.

• sage -t --warn-long 54.2 src/sage/rings/complex_mpc.pyx # 8 doctests failed and sage -t --warn-long 54.2 src/sage/rings/complex_arb.pyx # 7 doctests failed

  • coercion errors: ValueError: Cannot coerce algebraic number with non-zero imaginary part to algebraic real
  • precision regressions.

• sage -t --warn-long 54.2 src/sage/rings/qqbar.py # 6 doctests failed

  common parent issue: TypeError: unsupported operand parent(s) for '+': 'Algebraic Field' and 'Number Field in I with defining polynomial x^2 + 1'

• sage -t --warn-long 54.2 src/sage/rings/number_field/number_field_element_quadratic.pyx # 1 doctest failed

  common parent issue: TypeError: unsupported operand parent(s) for '*': 'Number Field in I with defining polynomial x^2 + 1' and 'Number Field in sqrt3 with defining polynomial x^2 - 3'

• sage -t --warn-long 54.2 src/sage/rings/polynomial/polynomial_rational_flint.pyx # 1 doctest failed

  false error is raised

• sage -t --warn-long 54.2 src/sage/rings/polynomial/cyclotomic.pyx # 2 doctests failed

  both can be fixed directly.

• Stuff in sage/geometry/hyperbolic_space/hyperbolic_*.py breaks down:

  • sage -t --warn-long 51.1 src/sage/geometry/hyperbolic_space/hyperbolic_point.py # 1 doctest failed
  • sage -t --warn-long 51.1 src/sage/geometry/hyperbolic_space/hyperbolic_geodesic.py # 4 doctests failed
  • sage -t --warn-long 51.1 src/sage/geometry/hyperbolic_space/hyperbolic_isometry.py # 1 doctest failed
  • sage -t --warn-long 51.1 src/sage/geometry/hyperbolic_space/hyperbolic_model.py # 2 doctests failed

  Not sure about these errors, some seem to be coercion related, others are just TypeError: 'sage.rings.complex_number.ComplexNumber' object is not callable---maybe that goes away along the way.

These are certainly not all failures, but they should give an idea of what breaks down. The biggest issue seems to be coercion...

[jdemeyer]

comment:25

Replying to @behackl:

The biggest issue seems to be coercion...

And stuff like I.log().

[rwst]

comment:26

Replying to @behackl:

• sage -t --warn-long 54.2 src/sage/rings/infinity.py # 2 doctests failed

  arithmetic with oo is broken.

It's just missing coercion of number field elements into the infinity ring. It would give a SignError anyway.

[videlec]

comment:69

Replying to @vbraun:

Merge conflict

How many times are we supposed to play this game? :)

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:

11b239c	#18036 add a dedicated element class for ℚ[i]
5ed00bc	#18036 explicit pushout(InfinityRing, *) -> SR
c9b08f6	#18036 fix imports of I from symbolic.all
11be379	#18036 fix type test in EllipticCurve_rational_field.eval_modular_form()
8125051	#18036 minor code adaptations in rings/
e5f9325	#18036 doctest updates: I.pyobject()
ed58a73	#18036 doctest updates: I → SR(I)
fe14b1a	#18036 doctest updates: pari-gp interfaces
671eed0	#18036 doctest updates: modular forms
10c5abf	#18036 doctest updates: misc benign changes

[sagetrac-git]

Changed commit from f885b3d to 10c5abf

[vbraun]

comment:73

PDF docs don't build

[fchapoton]

comment:74

could be because of

+    An element of ℚ[i].

[sagetrac-git]

Changed commit from 10c5abf to 54a34a7

[sagetrac-git]

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

54a34a7

#18036 fix docstring

[mezzarobba]

comment:76

Thanks Volker and Frédéric.

[vbraun]

Changed branch from u/mmezzarobba/18036-QQi to 54a34a7