#30360 closed defect (fixed)

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

Reported by: gh-kliem Owned by:
Priority: major Milestone: sage-9.2
Component: coercion Keywords: action, scalar, number field, free module
Cc: vdelecroix, tscrim Merged in:
Authors: Matthias Koeppe, Francis Clarke Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: a3c790f (Commits, GitHub, GitLab) Commit: a3c790f6fb38fca37c37dae86e568a5acdedb1c0
Dependencies: #30372 Stopgaps:

Status badges

Description

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.

Change History (44)

comment:1 Changed 16 months ago by mkoeppe

  • Cc vdelecroix tscrim added

comment:2 Changed 16 months ago by mkoeppe

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?

comment:3 follow-up: Changed 16 months ago by 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)

comment:4 Changed 16 months ago by gh-kliem

  • Summary changed from Pushout of number field and free module over ZZ fails to Action of number field on free module over ZZ not discovered

comment:5 Changed 16 months ago by gh-kliem

  • Keywords action scalar added; pushout removed

comment:6 Changed 16 months ago by mkoeppe

  • Branch set to u/mkoeppe/action_of_number_field_on_free_module_over_zz_not_discovered

comment:7 in reply to: ↑ 3 Changed 16 months ago by mkoeppe

  • Commit set to 8a9c2aa05454dc3695a1f8eceb4b657047fda489

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:

8a9c2aaWIP: NumberFieldElement_quadratic._lmul_, _rmul_: Remove

comment:8 Changed 16 months ago by mkoeppe

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.

comment:9 Changed 16 months ago by mkoeppe

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

Last edited 16 months ago by mkoeppe (previous) (diff)

comment:10 Changed 16 months ago by mkoeppe

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
Last edited 16 months ago by mkoeppe (previous) (diff)

comment:11 Changed 16 months ago by mkoeppe

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

comment:12 Changed 16 months ago by mkoeppe

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...

comment:13 Changed 16 months ago by mkoeppe

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)

comment:14 Changed 16 months ago by mkoeppe

Also

sage: K.<a> = NumberField(x^3 + 2/3*x + 1, latex_name='mangrove') 
sage: pushout(K, ZZ^2).base() is K                                                                                            
False
Last edited 16 months ago by mkoeppe (previous) (diff)

comment:15 Changed 16 months ago by mkoeppe

Looks like NumberField_generic.construction needs to handle latex_variable_names.

comment:16 Changed 16 months ago by mkoeppe

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'

comment:17 Changed 16 months ago by mkoeppe

  • Dependencies set to #30372

comment:18 Changed 16 months ago by git

  • Commit changed from 8a9c2aa05454dc3695a1f8eceb4b657047fda489 to 7a88492013d4b4147c35d0ceb2124e38bc393aae

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

fc48d6dsage.categories.pushout.AlgebraicExtensionFunctor: Handle latex_names
d85068fCommutativeRing.extension, NumberField_generic.extension, FiniteField.extension: Accept latex_name argument
7a88492NumberField_generic.construction: Pass latex_names to AlgebraicExtensionFunctor

comment:19 Changed 16 months ago by mkoeppe

  • Authors set to Matthias Koeppe

comment:20 Changed 16 months ago by git

  • Commit changed from 7a88492013d4b4147c35d0ceb2124e38bc393aae to 20ce24899b75292cec183f6c6f8c4af8c13fbcfc

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

1842e47sage.rings.number_field.number_field.NumberField_generic: Use _latex_names instead of __latex_variable_name, deprecate .latex_variable_name()
f9ceef7Merge 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
56f6c4dsage.rings.number_field.number_field.NumberFieldTower: Pass latex_name to extension
42eb272sage.rings.number_field.number_field.NumberField_generic.construction: Collect latex_names from the tower of extensions
6697bcaFixup
a6e828eMerge 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
20ce248src/sage/rings/number_field/number_field_element_quadratic.pyx: Add test for #30360

comment:21 Changed 16 months ago by mkoeppe

  • Status changed from new to needs_review

comment:22 Changed 16 months ago by git

  • Commit changed from 20ce24899b75292cec183f6c6f8c4af8c13fbcfc to 6d293911c4be27878476d9c057d70d5aa59f3305

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

da551dcsrc/sage/rings/number_field/number_field.py: Fix error found by pyflakes
6d29391Merge 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

comment:23 Changed 16 months ago by mkoeppe

The test failure reported by the patchbot seems unrelated. Ready for review

comment:24 Changed 16 months ago by gh-kliem

The test failure is subject of #29989, which is now closed.

comment:25 Changed 16 months ago by tscrim

It is a little strange that extension() has the parameters name and names but only latex_name. Although this isn't a dealbreaker, I think it would be good to have this consistent.

comment:26 Changed 16 months ago by mkoeppe

Thanks. Note that this is a comment for the dependency #30372. I have just found an old ticket with a similar goal, #19657. I am going to make the improvement regarding latex_name, latex_names on that ticket, reworking it on top of #30372.

comment:27 Changed 16 months ago by mkoeppe

Sorry, I was confused.

comment:28 Changed 16 months ago by mkoeppe

  • Status changed from needs_review to needs_work

comment:29 Changed 16 months ago by git

  • Commit changed from 6d293911c4be27878476d9c057d70d5aa59f3305 to c598fbbd9730a35ec17cb0c72b3f8f1608fcadf2

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

45aeabddoctests
f2ffd17Ring.extension, NumberField...__init__: Prefer latex_names over latex_name
e0c216bTreat latex_names in a similar manner to names
86b40c2adjust _latex for elements of relative number fields
3e5cc17src/sage/categories/pushout.py: Prefer latex_names in doctest
e40588aFiniteField.extension: Make arguments latex_name, latex_names keyword-only
c598fbbsrc/sage/rings/number_field/number_field_element.pyx: Use latex_variable_names instead of deprecated latex_variable_name

comment:30 Changed 16 months ago by mkoeppe

  • Authors changed from Matthias Koeppe to Matthias Koeppe, Francis Clarke
  • Status changed from needs_work to needs_review

Cherry-picked some more latex-name changes from #19657.

comment:31 Changed 16 months ago by mkoeppe

  • Summary changed from Action of number field on free module over ZZ not discovered to Fix multiplication of number field element * ZZ-vector by handling latex names of number field generators

comment:32 Changed 16 months ago by tscrim

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].

comment:33 Changed 16 months ago by mkoeppe

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

comment:34 Changed 16 months ago by tscrim

  • Reviewers set to Travis Scrimshaw

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

comment:35 Changed 16 months ago by mkoeppe

  • Status changed from needs_review to positive_review

Thanks!

comment:36 Changed 16 months ago by vbraun

  • Status changed from positive_review to needs_work

PDF docs don't build

comment:37 Changed 16 months ago by gh-kliem

@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.

comment:38 Changed 16 months ago by mkoeppe

  • Status changed from needs_work to positive_review

comment:39 Changed 16 months ago by vbraun

  • Status changed from positive_review to needs_work

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

comment:40 Changed 16 months ago by vbraun

[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.

comment:41 follow-up: Changed 16 months ago by tscrim

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

comment:42 in reply to: ↑ 41 Changed 16 months ago by tscrim

  • Branch changed from u/mkoeppe/action_of_number_field_on_free_module_over_zz_not_discovered to public/coercion/latex_names-30360
  • Commit changed from c598fbbd9730a35ec17cb0c72b3f8f1608fcadf2 to a3c790f6fb38fca37c37dae86e568a5acdedb1c0
  • Status changed from needs_work to needs_review

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:

45e0c4fMerge 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
a3c790fFix pdf docbuild in NumberFieldTower.

comment:43 Changed 16 months ago by mkoeppe

  • Status changed from needs_review to positive_review

Thanks, this did the job.

comment:44 Changed 15 months ago by vbraun

  • Branch changed from public/coercion/latex_names-30360 to a3c790f6fb38fca37c37dae86e568a5acdedb1c0
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.