Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid _element_constructor in padics #23884

Closed
jdemeyer opened this issue Sep 18, 2017 · 21 comments
Closed

Avoid _element_constructor in padics #23884

jdemeyer opened this issue Sep 18, 2017 · 21 comments

Comments

@jdemeyer
Copy link

See #23880.

CC: @simon-king-jena @koffie

Component: padics

Author: Jeroen Demeyer

Branch/Commit: 6878e67

Reviewer: David Roe, Travis Scrimshaw

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

@jdemeyer jdemeyer added this to the sage-8.1 milestone Sep 18, 2017
@simon-king-jena
Copy link
Member

comment:1

Why not do this change at #23881 or #23880?

@jdemeyer
Copy link
Author

comment:2

I like small self-contained tickets, not patchbombs.

@jdemeyer
Copy link
Author

comment:3

Also because padics is one of the few places outside of the parent/element/coercion framework where _element_constructor is directly called.

@jdemeyer
Copy link
Author

@jdemeyer
Copy link
Author

comment:5

Branch does not work :-(


New commits:

8f26c2dAvoid _element_constructor in padics

@jdemeyer
Copy link
Author

Commit: 8f26c2d

@simon-king-jena
Copy link
Member

comment:6

Replying to @jdemeyer:

Branch does not work :-(


New commits:

8f26c2dAvoid _element_constructor in padics

When merging it with #23881, at least the tests in sage.structure.parent work (mainly)...

What is failing?

@simon-king-jena
Copy link
Member

comment:7

Replying to @simon-king-jena:

What is failing?

In sage.rings.padics, not much is failing actually:

sage -t src/sage/rings/padics/padic_base_leaves.py  # 6 doctests failed
sage -t src/sage/rings/padics/padic_generic_element.pyx  # 1 doctest failed
sage -t src/sage/rings/padics/padic_ZZ_pX_FM_element.pyx  # 5 doctests failed

namely

sage -t src/sage/rings/padics/padic_base_leaves.py
**********************************************************************
File "src/sage/rings/padics/padic_base_leaves.py", line 260, in sage.rings.padics.padic_base_leaves.pAdicRingCappedRelative._coerce_map_from_
Failed example:
    K.has_coerce_map_from(int)
Expected:
    True
Got:
    False
**********************************************************************
File "src/sage/rings/padics/padic_base_leaves.py", line 336, in sage.rings.padics.padic_base_leaves.pAdicRingCappedAbsolute._coerce_map_from_
Failed example:
    K.has_coerce_map_from(int)
Expected:
    True
Got:
    False
**********************************************************************
File "src/sage/rings/padics/padic_base_leaves.py", line 415, in sage.rings.padics.padic_base_leaves.pAdicRingFloatingPoint._coerce_map_from_
Failed example:
    K.has_coerce_map_from(int)
Expected:
    True
Got:
    False
**********************************************************************
File "src/sage/rings/padics/padic_base_leaves.py", line 489, in sage.rings.padics.padic_base_leaves.pAdicRingFixedMod._coerce_map_from_
Failed example:
    K.has_coerce_map_from(int)
Expected:
    True
Got:
    False
**********************************************************************
File "src/sage/rings/padics/padic_base_leaves.py", line 595, in sage.rings.padics.padic_base_leaves.pAdicFieldCappedRelative._coerce_map_from_
Failed example:
    K.has_coerce_map_from(int)
Expected:
    True
Got:
    False
**********************************************************************
File "src/sage/rings/padics/padic_base_leaves.py", line 700, in sage.rings.padics.padic_base_leaves.pAdicFieldFloatingPoint._coerce_map_from_
Failed example:
    K.has_coerce_map_from(int)
Expected:
    True
Got:
    False
**********************************************************************
sage -t src/sage/rings/padics/padic_generic_element.pyx
**********************************************************************
File "src/sage/rings/padics/padic_generic_element.pyx", line 2577, in sage.rings.padics.padic_generic_element.pAdicGenericElement.exp
Failed example:
    z.log().exp()
Exception raised:
    Traceback (most recent call last):
      File "/home/king/Sage/git/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 515, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/king/Sage/git/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 885, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.rings.padics.padic_generic_element.pAdicGenericElement.exp[47]>", line 1, in <module>
        z.log().exp()
      File "sage/rings/padics/padic_generic_element.pyx", line 2181, in sage.rings.padics.padic_generic_element.pAdicGenericElement.log (build/cythonized/sage/rin)
        total = R.zero()
      File "sage/rings/ring.pyx", line 710, in sage.rings.ring.Ring.zero (build/cythonized/sage/rings/ring.c:7798)
        x = self(0)
      File "sage/structure/parent.pyx", line 936, in sage.structure.parent.Parent.__call__ (build/cythonized/sage/structure/parent.c:9668)
        return mor._call_(x)
      File "sage/structure/coerce_maps.pyx", line 102, in sage.structure.coerce_maps.DefaultConvertMap._call_ (build/cythonized/sage/structure/coerce_maps.c:3790)
        raise
      File "sage/structure/coerce_maps.pyx", line 97, in sage.structure.coerce_maps.DefaultConvertMap._call_ (build/cythonized/sage/structure/coerce_maps.c:3678)
        return C._element_constructor(C, x)
      File "sage/rings/padics/padic_ZZ_pX_FM_element.pyx", line 240, in sage.rings.padics.padic_ZZ_pX_FM_element.pAdicZZpXFMElement.__init__ (build/cythonized/sag)
        poly = x.polynomial().list()
    AttributeError: 'int' object has no attribute 'polynomial'
**********************************************************************
sage -t src/sage/rings/padics/padic_ZZ_pX_FM_element.pyx
**********************************************************************
File "src/sage/rings/padics/padic_ZZ_pX_FM_element.pyx", line 699, in sage.rings.padics.padic_ZZ_pX_FM_element.pAdicZZpXFMElement.__pow__
Failed example:
    W(0)^0
Exception raised:
    Traceback (most recent call last):
      File "/home/king/Sage/git/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 515, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/king/Sage/git/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 885, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.rings.padics.padic_ZZ_pX_FM_element.pAdicZZpXFMElement.__pow__[11]>", line 1, in <module>
        W(Integer(0))**Integer(0)
      File "sage/rings/padics/padic_ZZ_pX_FM_element.pyx", line 717, in sage.rings.padics.padic_ZZ_pX_FM_element.pAdicZZpXFMElement.__pow__ (build/cythonized/sage)
        return self.parent(1)
      File "sage/structure/element.pyx", line 751, in sage.structure.element.Element.parent (build/cythonized/sage/structure/element.c:6987)
        return self._parent(x)
      File "sage/structure/parent.pyx", line 936, in sage.structure.parent.Parent.__call__ (build/cythonized/sage/structure/parent.c:9668)
        return mor._call_(x)
      File "sage/structure/coerce_maps.pyx", line 102, in sage.structure.coerce_maps.DefaultConvertMap._call_ (build/cythonized/sage/structure/coerce_maps.c:3790)
        raise
      File "sage/structure/coerce_maps.pyx", line 97, in sage.structure.coerce_maps.DefaultConvertMap._call_ (build/cythonized/sage/structure/coerce_maps.c:3678)
        return C._element_constructor(C, x)
      File "sage/rings/padics/padic_ZZ_pX_FM_element.pyx", line 240, in sage.rings.padics.padic_ZZ_pX_FM_element.pAdicZZpXFMElement.__init__ (build/cythonized/sag)
        poly = x.polynomial().list()
    AttributeError: 'int' object has no attribute 'polynomial'
**********************************************************************
File "src/sage/rings/padics/padic_ZZ_pX_FM_element.pyx", line 701, in sage.rings.padics.padic_ZZ_pX_FM_element.pAdicZZpXFMElement.__pow__
Failed example:
    W(0)^0 == W(1)
Exception raised:
    Traceback (most recent call last):
      File "/home/king/Sage/git/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 515, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/king/Sage/git/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 885, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.rings.padics.padic_ZZ_pX_FM_element.pAdicZZpXFMElement.__pow__[12]>", line 1, in <module>
        W(Integer(0))**Integer(0) == W(Integer(1))
      File "sage/rings/padics/padic_ZZ_pX_FM_element.pyx", line 717, in sage.rings.padics.padic_ZZ_pX_FM_element.pAdicZZpXFMElement.__pow__ (build/cythonized/sage)
        return self.parent(1)
      File "sage/structure/element.pyx", line 751, in sage.structure.element.Element.parent (build/cythonized/sage/structure/element.c:6987)
        return self._parent(x)
      File "sage/structure/parent.pyx", line 936, in sage.structure.parent.Parent.__call__ (build/cythonized/sage/structure/parent.c:9668)
        return mor._call_(x)
      File "sage/structure/coerce_maps.pyx", line 102, in sage.structure.coerce_maps.DefaultConvertMap._call_ (build/cythonized/sage/structure/coerce_maps.c:3790)
        raise
      File "sage/structure/coerce_maps.pyx", line 97, in sage.structure.coerce_maps.DefaultConvertMap._call_ (build/cythonized/sage/structure/coerce_maps.c:3678)
        return C._element_constructor(C, x)
      File "sage/rings/padics/padic_ZZ_pX_FM_element.pyx", line 240, in sage.rings.padics.padic_ZZ_pX_FM_element.pAdicZZpXFMElement.__init__ (build/cythonized/sag)
        poly = x.polynomial().list()
    AttributeError: 'int' object has no attribute 'polynomial'
**********************************************************************
File "src/sage/rings/padics/padic_ZZ_pX_FM_element.pyx", line 710, in sage.rings.padics.padic_ZZ_pX_FM_element.pAdicZZpXFMElement.__pow__
Failed example:
    type(W(0)^0) == type(W(0))
Exception raised:
    Traceback (most recent call last):
      File "/home/king/Sage/git/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 515, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/king/Sage/git/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 885, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.rings.padics.padic_ZZ_pX_FM_element.pAdicZZpXFMElement.__pow__[17]>", line 1, in <module>
        type(W(Integer(0))**Integer(0)) == type(W(Integer(0)))
      File "sage/rings/padics/padic_ZZ_pX_FM_element.pyx", line 717, in sage.rings.padics.padic_ZZ_pX_FM_element.pAdicZZpXFMElement.__pow__ (build/cythonized/sage)
        return self.parent(1)
      File "sage/structure/element.pyx", line 751, in sage.structure.element.Element.parent (build/cythonized/sage/structure/element.c:6987)
        return self._parent(x)
      File "sage/structure/parent.pyx", line 936, in sage.structure.parent.Parent.__call__ (build/cythonized/sage/structure/parent.c:9668)
        return mor._call_(x)
      File "sage/structure/coerce_maps.pyx", line 102, in sage.structure.coerce_maps.DefaultConvertMap._call_ (build/cythonized/sage/structure/coerce_maps.c:3790)
        raise
      File "sage/structure/coerce_maps.pyx", line 97, in sage.structure.coerce_maps.DefaultConvertMap._call_ (build/cythonized/sage/structure/coerce_maps.c:3678)
        return C._element_constructor(C, x)
      File "sage/rings/padics/padic_ZZ_pX_FM_element.pyx", line 240, in sage.rings.padics.padic_ZZ_pX_FM_element.pAdicZZpXFMElement.__init__ (build/cythonized/sag)
        poly = x.polynomial().list()
    AttributeError: 'int' object has no attribute 'polynomial'
**********************************************************************
File "src/sage/rings/padics/padic_ZZ_pX_FM_element.pyx", line 815, in sage.rings.padics.padic_ZZ_pX_FM_element.pAdicZZpXFMElement._div_
Failed example:
    1 / W(14) == ~W(14)
Exception raised:
    Traceback (most recent call last):
      File "/home/king/Sage/git/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 515, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/king/Sage/git/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 885, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.rings.padics.padic_ZZ_pX_FM_element.pAdicZZpXFMElement._div_[5]>", line 1, in <module>
        Integer(1) / W(Integer(14)) == ~W(Integer(14))
      File "sage/rings/integer.pyx", line 1875, in sage.rings.integer.Integer.__div__ (build/cythonized/sage/rings/integer.c:13023)
        return coercion_model.bin_op(left, right, operator.div)
      File "sage/structure/coerce.pyx", line 1080, in sage.structure.coerce.CoercionModel_cache_maps.bin_op (build/cythonized/sage/structure/coerce.c:9419)
        action = self.get_action(xp, yp, op, x, y)
      File "sage/structure/coerce.pyx", line 1627, in sage.structure.coerce.CoercionModel_cache_maps.get_action (build/cythonized/sage/structure/coerce.c:16738)
        action = self.discover_action(R, S, op, r, s)
      File "sage/structure/coerce.pyx", line 1841, in sage.structure.coerce.CoercionModel_cache_maps.discover_action (build/cythonized/sage/structure/coerce.c:196)
        K = S._pseudo_fraction_field()
      File "sage/rings/ring.pyx", line 1374, in sage.rings.ring.CommutativeRing._pseudo_fraction_field (build/cythonized/sage/rings/ring.c:13025)
        return coercion_model.division_parent(self)
      File "sage/structure/coerce.pyx", line 959, in sage.structure.coerce.CoercionModel_cache_maps.division_parent (build/cythonized/sage/structure/coerce.c:9162)
        cpdef division_parent(self, Parent P):
      File "sage/structure/coerce.pyx", line 994, in sage.structure.coerce.CoercionModel_cache_maps.division_parent (build/cythonized/sage/structure/coerce.c:9052)
        ret = parent(~P.an_element())
      File "sage/rings/padics/padic_ZZ_pX_FM_element.pyx", line 493, in sage.rings.padics.padic_ZZ_pX_FM_element.pAdicZZpXFMElement.__invert__ (build/cythonized/s)
        raise ValueError("cannot invert non-unit")
    ValueError: cannot invert non-unit
**********************************************************************
File "src/sage/rings/padics/padic_ZZ_pX_FM_element.pyx", line 875, in sage.rings.padics.padic_ZZ_pX_FM_element.pAdicZZpXFMElement.is_zero
Failed example:
    O(w^189).is_zero()
Exception raised:
    Traceback (most recent call last):
      File "/home/king/Sage/git/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 515, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/king/Sage/git/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 885, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.rings.padics.padic_ZZ_pX_FM_element.pAdicZZpXFMElement.is_zero[4]>", line 1, in <module>
        O(w**Integer(189)).is_zero()
      File "/home/king/Sage/git/sage/local/lib/python2.7/site-packages/sage/rings/big_oh.py", line 152, in O
        return x.parent()(0, absprec=x.valuation(), **kwds)
      File "sage/structure/parent.pyx", line 938, in sage.structure.parent.Parent.__call__ (build/cythonized/sage/structure/parent.c:9695)
        return mor._call_with_args(x, args, kwds)
      File "sage/structure/coerce_maps.pyx", line 131, in sage.structure.coerce_maps.DefaultConvertMap._call_with_args (build/cythonized/sage/structure/coerce_map)
        raise
      File "sage/structure/coerce_maps.pyx", line 121, in sage.structure.coerce_maps.DefaultConvertMap._call_with_args (build/cythonized/sage/structure/coerce_map)
        return C._element_constructor(C, x, **kwds)
      File "sage/rings/padics/padic_ZZ_pX_FM_element.pyx", line 240, in sage.rings.padics.padic_ZZ_pX_FM_element.pAdicZZpXFMElement.__init__ (build/cythonized/sag)
        poly = x.polynomial().list()
    AttributeError: 'int' object has no attribute 'polynomial'
**********************************************************************

As it seems, int conversion has to be fixed. Strange. If ZZ works, int should work as well...

@simon-king-jena
Copy link
Member

comment:8

Odd.

sage: K = Zp(17)
sage: K.has_coerce_map_from(ZZ)
True
sage: K.has_coerce_map_from(int)
False
sage: ZZ.has_coerce_map_from(int)
True
sage: K(int(1))
1 + O(17^20)
sage: K(1)
1 + O(17^20)

Conversion works and coercion is supposed to be transitive.

@simon-king-jena
Copy link
Member

comment:9
sage: R = ZpFM(5,5)
sage: S.<x> = R[]
sage: f = x^5 + 75*x^3 - 15*x^2 +125*x - 5
sage: W.<w> = R.ext(f)
sage: W(1)
1 + O(w^25)
sage: W(int(1))
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-7-a7b09989f4cb> in <module>()
----> 1 W(int(Integer(1)))

/home/king/Sage/git/sage/src/sage/structure/parent.pyx in sage.structure.parent.Parent.__call__ (build/cythonized/sage/structure/parent.c:9668)()
    934         if mor is not None:
    935             if no_extra_args:
--> 936                 return mor._call_(x)
    937             else:
    938                 return mor._call_with_args(x, args, kwds)

/home/king/Sage/git/sage/src/sage/structure/coerce_maps.pyx in sage.structure.coerce_maps.DefaultConvertMap._call_ (build/cythonized/sage/structure/coerce_maps.c:3790)()
    100                 print(type(C), C)
    101                 print(type(C._element_constructor), C._element_constructor)
--> 102             raise
    103 
    104     cpdef Element _call_with_args(self, x, args=(), kwds={}):

/home/king/Sage/git/sage/src/sage/structure/coerce_maps.pyx in sage.structure.coerce_maps.DefaultConvertMap._call_ (build/cythonized/sage/structure/coerce_maps.c:3678)()
     95         cdef Parent C = self._codomain
     96         try:
---> 97             return C._element_constructor(C, x)
     98         except Exception:
     99             if print_warnings:

/home/king/Sage/git/sage/src/sage/rings/padics/padic_ZZ_pX_FM_element.pyx in sage.rings.padics.padic_ZZ_pX_FM_element.pAdicZZpXFMElement.__init__ (build/cythonized/sage/rings/padics/padic_ZZ_pX_FM_element.cpp:5027)()
    238             # Should only reach here if x is not in F_p
    239             z = parent.gen()
--> 240             poly = x.polynomial().list()
    241             x = sum([poly[i].lift() * (z ** i) for i in range(len(poly))], parent.zero())
    242         elif isinstance(x, ntl_ZZ_p):

AttributeError: 'int' object has no attribute 'polynomial'

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 19, 2017

Changed commit from 8f26c2d to 4a498b1

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 19, 2017

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

4a498b1Avoid _element_constructor in padics

@jdemeyer

This comment has been minimized.

@tscrim
Copy link
Collaborator

tscrim commented Sep 19, 2017

comment:13

Instead of using .Element, I would use .element_class:

sage: R = ZpCA(11, 5)
sage: R.Element
<type 'sage.rings.padics.padic_capped_absolute_element.pAdicCappedAbsoluteElement'>
sage: R.element_class
<type 'sage.rings.padics.padic_capped_absolute_element.pAdicCappedAbsoluteElement'>

for consistency with other parts of Sage and for robustness (in case Element becomes a Python class).

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 20, 2017

Changed commit from 4a498b1 to 6878e67

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 20, 2017

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

6878e67Avoid _element_constructor in padics

@jdemeyer
Copy link
Author

comment:15

Done.

@roed314
Copy link
Contributor

roed314 commented Sep 20, 2017

Reviewer: David Roe

@roed314
Copy link
Contributor

roed314 commented Sep 20, 2017

comment:16

Looks fine to me.

@tscrim
Copy link
Collaborator

tscrim commented Sep 20, 2017

Changed reviewer from David Roe to David Roe, Travis Scrimshaw

@vbraun
Copy link
Member

vbraun commented Sep 22, 2017

Changed branch from u/jdemeyer/avoid__element_constructor_in_padics to 6878e67

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants