Opened 4 years ago

Closed 4 years ago

#23534 closed enhancement (fixed)

Rational reconstruct over a generic univariate polynomial ring

Reported by: edgarcosta Owned by:
Priority: major Milestone: sage-8.1
Component: commutative algebra Keywords: sd87, days88
Cc: Merged in:
Authors: Edgar Costa Reviewers: Aly Deines, Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: f392803 (Commits, GitHub, GitLab) Commit: f3928035402903290092fcca27f40c686950dd71
Dependencies: Stopgaps:

Status badges

Description

I have implemented rational reconstruct over a generic univariate polynomial ring.

I'm not sure if I put in the right place, if not, please let me know.

Change History (27)

comment:1 Changed 4 years ago by edgarcosta

  • Status changed from new to needs_review

comment:2 Changed 4 years ago by edgarcosta

  • Keywords sd87 added

comment:3 Changed 4 years ago by git

  • Commit changed from f47d174fa2193e8972638305dcac31ba06882e56 to fd74a3cb0d182ad72dfbf6d974e143b28f6fff12

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

fd74a3cdoctest conventions

comment:4 Changed 4 years ago by git

  • Commit changed from fd74a3cb0d182ad72dfbf6d974e143b28f6fff12 to b03a92d4fbc05babc4d14ec4013ed6a277862594

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

b03a92dauthor line

comment:5 Changed 4 years ago by chapoton

ratioanal recosntruction ?

comment:6 Changed 4 years ago by git

  • Commit changed from b03a92d4fbc05babc4d14ec4013ed6a277862594 to 2a4cfc1b520cbda23f05b8311deade0bd7df8dd2

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

2a4cfc1typos

comment:7 Changed 4 years ago by edgarcosta

upsy-daisy! typo fixed. Thanks.

comment:8 Changed 4 years ago by jen

  • Keywords days88 added

comment:9 Changed 4 years ago by aly.deines

  • Status changed from needs_review to needs_work

A couple more quick changes:

In the following, Where should be When:

{{{Returns a tuple of two polynomials (n, d) where self * d is congruent to n modulo m.

Where \deg(n) \leq n_deg and \deg(d) \leq d_deg, the default value of n_deg and d_deg is floor(\deg(m) - 1)/2).}}}

Also, in the documentation you need an empty line after each ::

Finally, it could be rebased on the newest development branch.

I'm currently applying and playing with the code.

comment:10 Changed 4 years ago by git

  • Commit changed from 2a4cfc1b520cbda23f05b8311deade0bd7df8dd2 to 28c52b898d39b13cb6d818c3bda6acfded5fc7af

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

3879f46fix white space
9e58841lllgramint -> qflllgram in exception
352cf22fixing doc build
a27fde1fixing doc syntax
1de49f6empty lines
1cb8f35reference pari
cea589ea missing fake traceback
436614cMerge branch 'u/edgarcosta/LLL_gram' of git://trac.sagemath.org/sage into develop
56763a1fixing the number of ls in qflllgram
28c52b8Merge branch 'u/edgarcosta/rational_reconstruct' of git://trac.sagemath.org/sage into t/23534

comment:11 Changed 4 years ago by edgarcosta

  • Branch changed from u/edgarcosta/rational_reconstruct to u/edgarcosta/rational_reconstruct2
  • Commit changed from 28c52b898d39b13cb6d818c3bda6acfded5fc7af to 2a4cfc1b520cbda23f05b8311deade0bd7df8dd2

sorry for the mess

comment:12 Changed 4 years ago by git

  • Commit changed from 2a4cfc1b520cbda23f05b8311deade0bd7df8dd2 to 454788df2150ea40f088297fe42bbe3874cd6dfa

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

454788dUpdated input and output doc

comment:13 Changed 4 years ago by git

  • Commit changed from 454788df2150ea40f088297fe42bbe3874cd6dfa to d812f351cfbe0d444ebc63b976433ffba2e49e89

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

d812f35empty lines after ::

comment:14 Changed 4 years ago by edgarcosta

  • Status changed from needs_work to needs_review

comment:15 Changed 4 years ago by git

  • Commit changed from d812f351cfbe0d444ebc63b976433ffba2e49e89 to 31dbad13396e212e2017d3163a8f4c2bb3c8a46b

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

31dbad1formatting

comment:16 Changed 4 years ago by git

  • Commit changed from 31dbad13396e212e2017d3163a8f4c2bb3c8a46b to f4123f00594ec2c52cd879811a8b25199b57c700

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

46a463emore formatting
d898ac2more formatting
f4123f0more formatting

comment:17 Changed 4 years ago by git

  • Commit changed from f4123f00594ec2c52cd879811a8b25199b57c700 to 4a9c6c1d1a6ec0db05b47e9290c428608c34cc39

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

4a9c6c1fixed some formatting and converted an test into an example

comment:18 Changed 4 years ago by aly.deines

  • Branch changed from u/edgarcosta/rational_reconstruct2 to u/aly.deines/rational_reconstruct2

comment:19 Changed 4 years ago by git

  • Commit changed from 4a9c6c1d1a6ec0db05b47e9290c428608c34cc39 to f5a7466a91e77a711c4fe8f0b6244a5ed92381cc

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

f5a7466Removed trailin white space.

comment:20 Changed 4 years ago by aly.deines

  • Reviewers set to Aly Deines
  • Status changed from needs_review to positive_review

Updated and trailing whitespace removed.

Played with it, looks good and all tests pass.

comment:21 Changed 4 years ago by tscrim

  • Reviewers changed from Aly Deines to Aly Deines, Travis Scrimshaw
  • Status changed from positive_review to needs_work

Missing (real) author name. Also, the one-line description and ALGORITHM: needs a period, and use ``None`` for code formating. Last, I might consider doing the one-line description as

        Return a tuple of two polynomials ``(n, d)``,
        where ``self * d`` is congruent to ``n`` modulo ``m`` and
        ``n.degree() <= n_deg`` and ``d.degree() <= d_deg``.

and maybe make the INPUT: a bit more concise (same for m_deg):

        - ``n_deg`` -- (optional) an integer; the default is `\lfloor (\deg(m) - 1)/2 \rfloor`

comment:22 Changed 4 years ago by git

  • Commit changed from f5a7466a91e77a711c4fe8f0b6244a5ed92381cc to fbc547608a4f0a0a8ac903136e6b69a9842e9b9c

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

fbc5476Small clean up in documentation.

comment:23 Changed 4 years ago by aly.deines

  • Authors set to Edgar Costa
  • Status changed from needs_work to needs_review

Travis, will you re-review this?


New commits:

fbc5476Small clean up in documentation.

comment:24 Changed 4 years ago by edgarcosta

Thanks Aly!

comment:25 Changed 4 years ago by tscrim

Period for the ALGORITHM: block. Once done, you can set a positive review on my behalf.

comment:26 Changed 4 years ago by edgarcosta

  • Branch changed from u/aly.deines/rational_reconstruct2 to u/edgarcosta/rational_reconstruct2
  • Commit changed from fbc547608a4f0a0a8ac903136e6b69a9842e9b9c to f3928035402903290092fcca27f40c686950dd71
  • Status changed from needs_review to positive_review

New commits:

f392803Period for the ALGORITHM: block.

comment:27 Changed 4 years ago by vbraun

  • Branch changed from u/edgarcosta/rational_reconstruct2 to f3928035402903290092fcca27f40c686950dd71
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.