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

Frobenius endomorphism creation fail for infinite ring extension #34692

Closed
kryzar opened this issue Oct 24, 2022 · 23 comments
Closed

Frobenius endomorphism creation fail for infinite ring extension #34692

kryzar opened this issue Oct 24, 2022 · 23 comments

Comments

@kryzar
Copy link
Contributor

kryzar commented Oct 24, 2022

I wish to instantiate the Frobenius endomorphism of the field Fq(X) seen as a ring over Fq[X]. The following raises an exception:

sage: Fq = GF(11)
sage: FqX.<X> = Fq[]
sage: k = Frac(FqX)
sage: i = Hom(FqX, k).natural_map()
sage: K = k.over(i)
sage: K.frobenius_endomorphism()
---------------------------------------------------------------------------
NotImplementedError                       Traceback (most recent call last)
Input In [43], in <cell line: 6>()
      4 i = Hom(FqX, k).natural_map()
      5 K = k.over(i)
----> 6 K.frobenius_endomorphism()

File $SAGE_ROOT/sage/src/sage/rings/ring.pyx:1428, in sage.rings.ring.CommutativeRing.frobenius_endomorphism()
   1426     """
   1427     from .morphism import FrobeniusEndomorphism_generic
-> 1428     return FrobeniusEndomorphism_generic(self, n)
   1429 
   1430 def derivation_module(self, codomain=None, twist=None):

File $SAGE_ROOT/sage/src/sage/rings/morphism.pyx:2929, in sage.rings.morphism.FrobeniusEndomorphism_generic.__init__()
   2927 if not isinstance(domain, CommutativeRing):
   2928     raise TypeError("The base ring must be a commutative ring")
-> 2929 self._p = domain.characteristic()
   2930 if not self._p.is_prime():
   2931     raise TypeError("the characteristic of the base ring must be prime")

File $SAGE_ROOT/sage/src/sage/categories/rings.py:588, in Rings.ParentMethods.characteristic(self)
    586 from sage.rings.infinity import infinity
    587 from sage.rings.integer_ring import ZZ
--> 588 order_1 = self.one().additive_order()
    589 return ZZ.zero() if order_1 is infinity else order_1

File $SAGE_ROOT/sage/src/sage/rings/ring_extension_element.pyx:415, in sage.rings.ring_extension_element.RingExtensionElement.additive_order()
    413         5
    414     """
--> 415     return self._backend.additive_order()
    416 
    417 def multiplicative_order(self):

File $SAGE_ROOT/sage/src/sage/structure/element.pyx:2818, in sage.structure.element.RingElement.additive_order()
   2816     Return the additive order of ``self``.
   2817     """
-> 2818     raise NotImplementedError
   2819 
   2820 def multiplicative_order(self):

NotImplementedError:

Best,

Antoine Leudière

CC: @DavidAyotte @xcaruso

Component: algebra

Author: Antoine Leudière

Branch/Commit: ab2c211

Reviewer: Xavier Caruso, David Ayotte

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

@kryzar kryzar added this to the sage-9.8 milestone Oct 24, 2022
@DavidAyotte
Copy link
Member

comment:1

Hi Antoine! I think that to make this works, you need to implement the characteristic method of a ring over a base.

@DavidAyotte
Copy link
Member

comment:2

I investigated a bit further and, to make your above code work, I think that you simply need to implement the characteristic method for the class sage.rings.ring_extensions.RingExtension_generic:

    def characteristic(self):
        return self._backend.characteristic()

I cc'ed Xavier here because he implemented extensions of rings and therefore he might know a better way to solve this.

@kryzar
Copy link
Contributor Author

kryzar commented Oct 28, 2022

comment:3

Thank you David for the input. I am happy to do it!

A.

@kryzar
Copy link
Contributor Author

kryzar commented Nov 14, 2022

Branch: public/ring-extension-characteristic

@kryzar
Copy link
Contributor Author

kryzar commented Nov 14, 2022

Commit: 501a254

@kryzar
Copy link
Contributor Author

kryzar commented Nov 14, 2022

comment:4

Here is a proposal. Let me know what you think.

Best,
Antoine


New commits:

501a254Implement `characteristic` method for rings extensions

@DavidAyotte
Copy link
Member

comment:5

Very minor detail: in the output field you could write instead

A prime number or zero.

Also, I think that sometimes it is acceptable to omit the output field in the documentation when the output is relatively straightforward (but you can keep it like that, it's perfectly fine too).

If you are happy with your changes you can put your ticket in "need review". The next step would be to wait for the build actions to finish.

@DavidAyotte
Copy link
Member

Author: Antoine Leudière

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 14, 2022

Changed commit from 501a254 to bcffa0c

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 14, 2022

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

bcffa0cFix imprecision in documentation

@kryzar
Copy link
Contributor Author

kryzar commented Nov 14, 2022

comment:7

Replying to David Ayotte:

Very minor detail: in the output field you could write instead

A prime number or zero.

I am so stupid.

Also, I think that sometimes it is acceptable to omit the output field in the documentation when the output is relatively straightforward (but you can keep it like that, it's perfectly fine too).

I think this can help resolve any ambiguities.

If you are happy with your changes you can put your ticket in "need review". The next step would be to wait for the build actions to finish.

Thanks a lot; it was mostly your work (comment [comment:2]).

Best,

Antoine


New commits:

bcffa0cFix imprecision in documentation

@xcaruso
Copy link
Contributor

xcaruso commented Nov 14, 2022

comment:9

Maybe, you should also have an example B.over(A) where A and B have different characteristic.
For example, I suggest to replace your last example by

    sage: E = Fp(7).over(ZZ)
    sage: E.characteristic()
    7

Also you should add a doctest (in the section TESTS) demonstrating that the bug noticed in this ticket is fixed.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 14, 2022

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

675e249Add tests for characteristic method

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 14, 2022

Changed commit from bcffa0c to 675e249

@xcaruso
Copy link
Contributor

xcaruso commented Nov 14, 2022

comment:11

For the last example, you don't need to create i. The following code works well:

sage: Fq = GF(11)
sage: FqX.<X> = Fq[]
sage: k = Frac(FqX)
sage: K = k.over(FqX)
sage: K.frobenius_endomorphism()

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 14, 2022

Changed commit from 675e249 to ab2c211

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 14, 2022

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

ab2c211Fix doctest

@xcaruso
Copy link
Contributor

xcaruso commented Nov 14, 2022

comment:13

Looks good to me. Waiting for the patchbot.

@DavidAyotte
Copy link
Member

comment:14

Looks good to me too! The failed doctest of the patchbot is definitely not related to this ticket and all the Github actions passed.

@DavidAyotte
Copy link
Member

Reviewer: Xavier Caruso, David Ayotte

@kryzar
Copy link
Contributor Author

kryzar commented Nov 15, 2022

comment:15

Copy that! Thank you for the review.

Does that mean I can merge this branch into that of Drinfeld modules? I need the patch to fix some bugs. Or should I wait for something else?

Best,

Antoine

@DavidAyotte
Copy link
Member

comment:16

You can merge it an add this ticket number in the "dependencies" field of your ticket on Drinfeld modules. For more info, see https://doc.sagemath.org/html/en/developer/trac.html#the-ticket-fields

@vbraun
Copy link
Member

vbraun commented Nov 20, 2022

Changed branch from public/ring-extension-characteristic to ab2c211

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

4 participants