Fix multiplication of number field element * ZZ-vector by handling latex names of number field generators #30360

kliem · 2020-08-14T20:21:18Z

sage: 1/2*vector([1,2,3])
(1/2, 1, 3/2)

sage: AA(2).sqrt()*vector([1,2,3])                                                                                                                                                                                                                                                                                                                                         
(1.414213562373095?, 2.828427124746190?, 4.242640687119285?)

sage: K.<sqrt5> = QuadraticField(5, embedding=AA(5).sqrt())                                                                                                                                                                                                                                                                                                                
sage: sqrt5*vector([1,2])                                                                                                                                                                                                                                                                                                                                                  
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-65-1ac790b433d6> in <module>
----> 1 sqrt5*vector([Integer(1),Integer(2)])

~/Applications/sage/local/lib/python3.7/site-packages/sage/structure/element.pyx in sage.structure.element.Element.__mul__ (build/cythonized/sage/structure/element.c:12196)()
   1513             return (<Element>left)._mul_(right)
   1514         if BOTH_ARE_ELEMENT(cl):
-> 1515             return coercion_model.bin_op(left, right, mul)
   1516 
   1517         cdef long value

~/Applications/sage/local/lib/python3.7/site-packages/sage/structure/coerce.pyx in sage.structure.coerce.CoercionModel.bin_op (build/cythonized/sage/structure/coerce.c:11304)()
   1246         # We should really include the underlying error.
   1247         # This causes so much headache.
-> 1248         raise bin_op_exception(op, x, y)
   1249 
   1250     cpdef canonical_coercion(self, x, y):

TypeError: unsupported operand parent(s) for *: 'Number Field in sqrt5 with defining polynomial x^2 - 5 with sqrt5 = 2.236067977499790?' and 'Ambient free module of rank 2 over the principal ideal domain Integer Ring'

It doesn't matter whether you specify the embedding or not.

Depends on #30372

CC: @videlec @tscrim

Component: coercion

Keywords: action, scalar, free module

Author: Matthias Koeppe, Francis Clarke

Branch/Commit: a3c790f

Reviewer: Travis Scrimshaw

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

The text was updated successfully, but these errors were encountered:

mkoeppe · 2020-08-14T23:37:23Z

comment:2

It's not really the pushout that's failing here:

sage: from sage.categories.pushout import pushout
sage: pushout(sqrt5.parent(), vector([1,2]).parent())
Vector space of dimension 2 over Number Field in sqrt5 with defining polynomial x^2 - 5 with sqrt5 = 2.236067977499790?

mkoeppe · 2020-08-14T23:53:15Z

comment:3

I think the methods _lmul_ and _rmul_ of NumberFieldElement_quadratic are in the way. I think they may just need to be removed (haven't tried)

kliem · 2020-08-15T11:24:18Z

Changed keywords from pushout, free module to action, scalar, free module

mkoeppe · 2020-08-15T16:21:00Z

Branch: u/mkoeppe/action_of_number_field_on_free_module_over_zz_not_discovered

mkoeppe · 2020-08-15T16:21:23Z

Commit: 8a9c2aa

mkoeppe · 2020-08-15T16:21:23Z

comment:7

Replying to @mkoeppe:

I think the methods _lmul_ and _rmul_ of NumberFieldElement_quadratic are in the way. I think they may just need to be removed (haven't tried)

By itself this does not fix it... (see branch)

New commits:

`8a9c2aa`	`WIP: NumberFieldElement_quadratic._lmul_, _rmul_: Remove`

mkoeppe · 2020-08-15T16:27:02Z

comment:8

You are right, the action is missing.

sage: cm.explain(AA, vector([1,2]).parent(), operator.mul)                                                                                                          
Action discovered.
    Left scalar multiplication by Algebraic Real Field on Ambient free module of rank 2 over the principal ideal domain Integer Ring
Result lives in Vector space of dimension 2 over Algebraic Real Field
Vector space of dimension 2 over Algebraic Real Field
sage: cm.explain(K, vector([1,2]).parent(), operator.mul)                                                                                                           

Unknown result parent.

mkoeppe · 2020-08-15T16:54:22Z

comment:9

It looks like this is because of a unique representation / equality bug for number fields

mkoeppe · 2020-08-15T17:00:35Z

comment:10

sage: K.<sqrt5> = QuadraticField(5, embedding=AA(5).sqrt())
sage: V = vector([1, 2]).parent()
sage: from sage.structure.coerce_actions import LeftModuleAction                                                                                                    
sage: LeftModuleAction(AA, V)
Left scalar multiplication by Algebraic Real Field on Ambient free module of rank 2 over the principal ideal domain Integer Ring
sage: LeftModuleAction(K, V)                                                                                                                               
CoercionException: Actor must be coercible into base.

Looking at LeftModuleAction.__init__, this boils down to:

sage: from sage.categories.pushout import pushout                                                         
sage: K                                                                                                   
Number Field in sqrt5 with defining polynomial x^2 - 5 with sqrt5 = 2.236067977499790?
sage: pushout(K, ZZ) is K                                                                                 
True
sage: K2 = pushout(K, V).base(); K2                                                                       
Number Field in sqrt5 with defining polynomial x^2 - 5 with sqrt5 = 2.236067977499790?
sage: K2 is K                                                                                             
False
sage: K2 == K                                                                                             
False

mkoeppe · 2020-08-15T17:06:06Z

comment:11

This bug seems specific to quadratic fields:

sage: K = QuadraticField(5)                                                                               
sage: pushout(K, ZZ^2).base() is K                                                                        
False
sage: K.<a> = NumberField(x^3 + 2/3*x + 1)                                                                
sage: pushout(K, ZZ^2).base() is K                                                                        
True
sage: K.<a> = NumberField(x^3 + 2/3*x + 1, embedding=AA(1))                                               
sage: pushout(K, ZZ^2).base() is K                                                                        
True

mkoeppe · 2020-08-15T17:27:52Z

comment:12

sage: QuadraticField(5).latex_variable_name()                                                                                       
'\\sqrt{5}'
sage: pushout(QuadraticField(5), ZZ^2).base().latex_variable_name()                                                                 
'a'

That's why they're not equal...

mkoeppe · 2020-08-15T17:32:16Z

comment:13

sage: K = QuadraticField(5, latex_name=None)                                                                                        
sage: pushout(K, ZZ^2).base() is K                                                                                                  
True
sage: K.<sqrt5> = QuadraticField(5, embedding=AA(5).sqrt(), latex_name=None)                                                                                                                                                                                                                                                                                                                
sage: sqrt5*vector([1,2])  
(sqrt5, 2*sqrt5)

mkoeppe · 2020-08-15T17:34:06Z

comment:14

Also

sage: K.<a> = NumberField(x^3 + 2/3*x + 1, latex_name='mangrove') 
sage: pushout(K, ZZ^2).base() is K                                                                                            
False

mkoeppe · 2020-08-15T17:39:26Z

comment:15

Looks like NumberField_generic.construction needs to handle latex_variable_names.

mkoeppe · 2020-08-15T18:49:00Z

comment:16

One more broken thing:

sage: K3.<a> = NumberField(x^3+x^2+1, latex_name=['alpha'])                                                                         
sage: K3.latex_variable_names()                                                                                                     
['a']
sage: K3.latex_variable_name()                                                                                                      
'alpha'

mkoeppe · 2020-08-15T19:06:44Z

Dependencies: #30372

sagetrac-git · 2020-08-15T19:12:01Z

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

`fc48d6d`	`sage.categories.pushout.AlgebraicExtensionFunctor: Handle latex_names`
`d85068f`	`CommutativeRing.extension, NumberField_generic.extension, FiniteField.extension: Accept latex_name argument`
`7a88492`	`NumberField_generic.construction: Pass latex_names to AlgebraicExtensionFunctor`

sagetrac-git · 2020-08-15T19:12:01Z

Changed commit from 8a9c2aa to 7a88492

mkoeppe · 2020-08-15T19:12:22Z

Author: Matthias Koeppe

sagetrac-git · 2020-08-15T20:28:15Z

Changed commit from 7a88492 to 20ce248

sagetrac-git · 2020-08-15T20:28:15Z

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

`1842e47`	`sage.rings.number_field.number_field.NumberField_generic: Use _latex_names instead of __latex_variable_name, deprecate .latex_variable_name()`
`f9ceef7`	`Merge branch 't/30372/replace_numberfield_generic___latex_variable_name_by___latex_name' into t/30360/action_of_number_field_on_free_module_over_zz_not_discovered`
`56f6c4d`	`sage.rings.number_field.number_field.NumberFieldTower: Pass latex_name to extension`
`42eb272`	`sage.rings.number_field.number_field.NumberField_generic.construction: Collect latex_names from the tower of extensions`
`6697bca`	`Fixup`
`a6e828e`	`Merge branch 't/30372/replace_numberfield_generic___latex_variable_name_by___latex_name' into t/30360/action_of_number_field_on_free_module_over_zz_not_discovered`
`20ce248`	`src/sage/rings/number_field/number_field_element_quadratic.pyx: Add test for #30360`

sagetrac-git · 2020-08-16T00:35:18Z

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

`da551dc`	`src/sage/rings/number_field/number_field.py: Fix error found by pyflakes`
`6d29391`	`Merge branch 't/30372/replace_numberfield_generic___latex_variable_name_by___latex_name' into t/30360/action_of_number_field_on_free_module_over_zz_not_discovered`

sagetrac-git · 2020-08-16T00:35:18Z

Changed commit from 20ce248 to 6d29391

tscrim · 2020-08-18T00:11:52Z

comment:32

I am not sure about this:

        if latex_name is None and latex_names is not None:
            latex_name = latex_names

I would expect latex_names to be a tuple, so I would think the correct thing to do would be latex_name = latex_names[0].

mkoeppe · 2020-08-18T00:19:40Z

comment:33

This is passed on to the extension method, which handles both singular and plural.

tscrim · 2020-08-18T00:22:49Z

Reviewer: Travis Scrimshaw

tscrim · 2020-08-18T00:22:49Z

comment:34

Okay, good. Then let's wait for the patchbot, but if that is green, then positive review.

mkoeppe · 2020-08-18T02:00:25Z

comment:35

Thanks!

vbraun · 2020-08-24T22:56:11Z

comment:36

PDF docs don't build

kliem · 2020-08-25T05:51:40Z

comment:37

@vbraun: Can you be more precice? Is this on a test branch on top of other tickets?

The patchbot exceeds disk quota due to ccache, so I don't think this has anything to do with this ticket.

For me, documentation builds just fine.

vbraun · 2020-08-26T22:54:59Z

comment:39

PDF docs don't build (non-pdf is ok)

vbraun · 2020-08-26T22:55:43Z

comment:40

[docpdf] 
[docpdf] ! Package inputenc Error: Unicode character ^^G (U+0007)
[docpdf] (inputenc)                not set up for use with LaTeX.
[docpdf] 
[docpdf] See the inputenc package documentation for explanation.
[docpdf] Type  H <return>  for immediate help.
[docpdf]  ...                                              
[docpdf]                                                   
[docpdf] l.1012 ...G{l+s+s1}{\PYGZsq{}}\PYG{p}{]}\PYG{p}{)}
[docpdf]                                                   
[docpdf] ? 
[docpdf] ! Emergency stop.

tscrim · 2020-08-27T00:02:49Z

comment:41

I think the problem is in NumberFieldTower, the string is not a r""". Let me see if that fixes it.

tscrim · 2020-08-27T00:43:53Z

Changed commit from c598fbb to a3c790f

tscrim · 2020-08-27T00:43:53Z

Changed branch from u/mkoeppe/action_of_number_field_on_free_module_over_zz_not_discovered to public/coercion/latex_names-30360

tscrim · 2020-08-27T00:43:53Z

comment:42

Replying to @tscrim:

I think the problem is in NumberFieldTower, the string is not a r""". Let me see if that fixes it.

I think that fixes it. Please double-check. What a cryptic error message through...

New commits:

`45e0c4f`	`Merge branch 'u/mkoeppe/action_of_number_field_on_free_module_over_zz_not_discovered' of git://trac.sagemath.org/sage into public/coercion/latex_names-30360`
`a3c790f`	`Fix pdf docbuild in NumberFieldTower.`

mkoeppe · 2020-08-27T00:54:29Z

comment:43

Thanks, this did the job.

vbraun · 2020-09-01T22:59:36Z

Changed branch from public/coercion/latex_names-30360 to a3c790f

kliem added this to the sage-9.2 milestone Aug 14, 2020

kliem added c: coercion labels Aug 14, 2020

kliem changed the title ~~Pushout of number field and free module over ZZ fails~~ Action of number field on free module over ZZ not discovered Aug 15, 2020

mkoeppe added the s: needs review label Aug 15, 2020

mkoeppe added s: positive review and removed s: needs review labels Aug 18, 2020

vbraun added s: needs work and removed s: positive review labels Aug 24, 2020

mkoeppe added s: positive review and removed s: needs work labels Aug 26, 2020

vbraun added s: needs work and removed s: positive review labels Aug 26, 2020

tscrim added s: needs review and removed s: needs work labels Aug 27, 2020

mkoeppe added s: positive review and removed s: needs review labels Aug 27, 2020

vbraun removed the s: positive review label Sep 1, 2020

vbraun closed this as completed in 1b9d7cf Sep 1, 2020

simon-king-jena mentioned this issue Jan 14, 2023

Let the TestSuite test that the construction of a parent returns the parent #15223

Closed

Fix multiplication of number field element * ZZ-vector by handling latex names of number field generators #30360

Fix multiplication of number field element * ZZ-vector by handling latex names of number field generators #30360

Comments

kliem commented Aug 14, 2020

mkoeppe commented Aug 14, 2020

mkoeppe commented Aug 14, 2020

kliem commented Aug 15, 2020

mkoeppe commented Aug 15, 2020

mkoeppe commented Aug 15, 2020

mkoeppe commented Aug 15, 2020

mkoeppe commented Aug 15, 2020

mkoeppe commented Aug 15, 2020

mkoeppe commented Aug 15, 2020

mkoeppe commented Aug 15, 2020

mkoeppe commented Aug 15, 2020

mkoeppe commented Aug 15, 2020

mkoeppe commented Aug 15, 2020

mkoeppe commented Aug 15, 2020

mkoeppe commented Aug 15, 2020

mkoeppe commented Aug 15, 2020

sagetrac-git mannequin commented Aug 15, 2020

sagetrac-git mannequin commented Aug 15, 2020

mkoeppe commented Aug 15, 2020

sagetrac-git mannequin commented Aug 15, 2020

sagetrac-git mannequin commented Aug 15, 2020

sagetrac-git mannequin commented Aug 16, 2020

sagetrac-git mannequin commented Aug 16, 2020

tscrim commented Aug 18, 2020

mkoeppe commented Aug 18, 2020

tscrim commented Aug 18, 2020

tscrim commented Aug 18, 2020

mkoeppe commented Aug 18, 2020

vbraun commented Aug 24, 2020

kliem commented Aug 25, 2020

vbraun commented Aug 26, 2020

vbraun commented Aug 26, 2020

tscrim commented Aug 27, 2020

tscrim commented Aug 27, 2020

tscrim commented Aug 27, 2020

tscrim commented Aug 27, 2020

mkoeppe commented Aug 27, 2020

vbraun commented Sep 1, 2020