Opened 4 years ago

Closed 12 months ago

#23174 closed enhancement (fixed)

p and p^n-th roots in function fields

Reported by: saraedum Owned by:
Priority: minor Milestone: sage-9.3
Component: commutative algebra Keywords: sd86.5
Cc: swewers Merged in:
Authors: Julian Rüth Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 3786aa1 (Commits, GitHub, GitLab) Commit: 3786aa143c10744bb9d9331fcad12b1a42a39fe4
Dependencies: #16561, #16564 Stopgaps:

Status badges

Change History (16)

comment:1 Changed 4 years ago by saraedum

  • Priority changed from major to minor

comment:2 Changed 3 years ago by saraedum

  • Description modified (diff)

comment:3 Changed 3 years ago by saraedum

  • Dependencies set to #16561, #16564

comment:4 Changed 14 months ago by saraedum

  • Branch set to u/saraedum/23174-roots-in-function-fields

comment:5 Changed 14 months ago by git

  • Commit set to a740a00b1fed89a6b522624451ee4f49c267d324

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

a740a00implement nth_root/is_nth_power for function field elements

comment:6 Changed 14 months ago by git

  • Commit changed from a740a00b1fed89a6b522624451ee4f49c267d324 to 9924602fcfdfea56e0926c33ec01e02cf8e9c7a0

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

9924602implement nth_root/is_nth_power for function field elements

comment:7 Changed 14 months ago by saraedum

  • Authors set to Julian Rüth
  • Description modified (diff)

comment:8 Changed 14 months ago by saraedum

  • Cc swewers added

comment:9 Changed 14 months ago by git

  • Commit changed from 9924602fcfdfea56e0926c33ec01e02cf8e9c7a0 to 9be299e2142e6569e0d1e145d9d9c255a2e49d3d

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

9be299eadd tests for is_nth_power

comment:10 Changed 14 months ago by saraedum

  • Status changed from new to needs_review

comment:11 Changed 14 months ago by saraedum

I did not run full doctests. Let's see what the patchbot thinks about this one.

comment:12 Changed 14 months ago by swewers

When I build the documentation there is an error complaining that the reference [GiTr1996] is introduced twice.

The code itself is fine. Thanks!

comment:13 Changed 14 months ago by tscrim

  • Branch changed from u/saraedum/23174-roots-in-function-fields to public/rings/roots_function_fields-23174
  • Commit changed from 9be299e2142e6569e0d1e145d9d9c255a2e49d3d to 3786aa143c10744bb9d9331fcad12b1a42a39fe4
  • Milestone changed from sage-8.0 to sage-9.3
  • Reviewers set to Travis Scrimshaw

I also made some reviewer changes. Basically they are all done to improve the C code generated by Cython. One thing that might be more controversial is my unrolling of

        for i in range(deg):
            v += x.list()
            x *= yp

I did this so you don't create as many temporary objects and to avoid the repeated calls to self._parent.degree() and self._parent.polynomial(). If you don't want the more complicated code, feel free to revert the last commit.


New commits:

c7ef172Fixing docbuild. Trying to optimize the _pth_root() implementation using Cython.
c62db94Doing some more work to improve the Cython code.
3786aa1Unrolling the x.list() and x *= yp in _pth_root() to avoid temporary objects and repeated function calls.

comment:14 Changed 14 months ago by saraedum

I don't mind these changes though I doubt that they make a difference.

Feel free to set this to positive review.

comment:15 Changed 14 months ago by tscrim

  • Status changed from needs_review to positive_review

Thank you.

comment:16 Changed 12 months ago by vbraun

  • Branch changed from public/rings/roots_function_fields-23174 to 3786aa143c10744bb9d9331fcad12b1a42a39fe4
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.