Opened 16 months ago
Closed 15 months ago
#30360 closed defect (fixed)
Fix multiplication of number field element * ZZvector by handling latex names of number field generators
Reported by:  ghkliem  Owned by:  

Priority:  major  Milestone:  sage9.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: 
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) <ipythoninput651ac790b433d6> in <module> > 1 sqrt5*vector([Integer(1),Integer(2)]) ~/Applications/sage/local/lib/python3.7/sitepackages/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/sitepackages/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
 Cc vdelecroix tscrim added
comment:2 Changed 16 months ago by
comment:3 followup: ↓ 7 Changed 16 months ago by
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
 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
 Keywords action scalar added; pushout removed
comment:6 Changed 16 months ago by
 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
 Commit set to 8a9c2aa05454dc3695a1f8eceb4b657047fda489
comment:8 Changed 16 months ago by
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
It looks like this is because of a unique representation / equality bug for number fields
comment:10 Changed 16 months ago by
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
comment:11 Changed 16 months ago by
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
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
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
Also
sage: K.<a> = NumberField(x^3 + 2/3*x + 1, latex_name='mangrove') sage: pushout(K, ZZ^2).base() is K False
comment:15 Changed 16 months ago by
Looks like NumberField_generic.construction
needs to handle latex_variable_names
.
comment:16 Changed 16 months ago by
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
 Dependencies set to #30372
comment:18 Changed 16 months ago by
 Commit changed from 8a9c2aa05454dc3695a1f8eceb4b657047fda489 to 7a88492013d4b4147c35d0ceb2124e38bc393aae
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

comment:19 Changed 16 months ago by
comment:20 Changed 16 months ago by
 Commit changed from 7a88492013d4b4147c35d0ceb2124e38bc393aae to 20ce24899b75292cec183f6c6f8c4af8c13fbcfc
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

comment:21 Changed 16 months ago by
 Status changed from new to needs_review
comment:22 Changed 16 months ago by
 Commit changed from 20ce24899b75292cec183f6c6f8c4af8c13fbcfc to 6d293911c4be27878476d9c057d70d5aa59f3305
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

comment:23 Changed 16 months ago by
The test failure reported by the patchbot seems unrelated. Ready for review
comment:24 Changed 16 months ago by
The test failure is subject of #29989, which is now closed.
comment:25 Changed 16 months ago by
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
comment:27 Changed 16 months ago by
Sorry, I was confused.
comment:28 Changed 16 months ago by
 Status changed from needs_review to needs_work
comment:29 Changed 16 months ago by
 Commit changed from 6d293911c4be27878476d9c057d70d5aa59f3305 to c598fbbd9730a35ec17cb0c72b3f8f1608fcadf2
Branch pushed to git repo; I updated commit sha1. New commits:
45aeabd  doctests

f2ffd17  Ring.extension, NumberField...__init__: Prefer latex_names over latex_name

e0c216b  Treat latex_names in a similar manner to names

86b40c2  adjust _latex for elements of relative number fields

3e5cc17  src/sage/categories/pushout.py: Prefer latex_names in doctest

e40588a  FiniteField.extension: Make arguments latex_name, latex_names keywordonly

c598fbb  src/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
 Status changed from needs_work to needs_review
Cherrypicked some more latexname changes from #19657.
comment:31 Changed 16 months ago by
 Summary changed from Action of number field on free module over ZZ not discovered to Fix multiplication of number field element * ZZvector by handling latex names of number field generators
comment:32 Changed 16 months ago by
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
This is passed on to the extension
method, which handles both singular and plural.
comment:34 Changed 16 months ago by
 Reviewers set to Travis Scrimshaw
Okay, good. Then let's wait for the patchbot, but if that is green, then positive review.
comment:36 Changed 16 months ago by
 Status changed from positive_review to needs_work
PDF docs don't build
comment:37 Changed 16 months ago by
@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
 Status changed from needs_work to positive_review
comment:39 Changed 16 months ago by
 Status changed from positive_review to needs_work
PDF docs don't build (nonpdf is ok)
comment:40 Changed 16 months ago by
[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 followup: ↓ 42 Changed 16 months ago by
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
 Branch changed from u/mkoeppe/action_of_number_field_on_free_module_over_zz_not_discovered to public/coercion/latex_names30360
 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 ar"""
. Let me see if that fixes it.
I think that fixes it. Please doublecheck. 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_names30360

a3c790f  Fix pdf docbuild in NumberFieldTower.

comment:43 Changed 16 months ago by
 Status changed from needs_review to positive_review
Thanks, this did the job.
comment:44 Changed 15 months ago by
 Branch changed from public/coercion/latex_names30360 to a3c790f6fb38fca37c37dae86e568a5acdedb1c0
 Resolution set to fixed
 Status changed from positive_review to closed
It's not really the pushout that's failing here: