Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#7836 closed defect (fixed)

Clean up the CRT_* functions

Reported by: mhansen Owned by: AlexGhitza
Priority: minor Milestone: sage-4.3.1
Component: basic arithmetic Keywords:
Cc: cremona, rlm Merged in: sage-4.3.1.alpha2
Authors: John Cremona Reviewers: Robert Miller
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by mhansen)

From #7595:

> I have some problems with the CRT* functions though.
> 
>    1. CRT_list does not check that the two lists have the same length;  if the moduli list is shorter you get an IndexError, but it would be better to catch that and raise a more informative error.
> 
>    2. CRT_basis is rather silly.   It calls CRT_list n times with the same moduli, which must be wasteful.  It would be better to call plain CRT n times with suitable moduli (exercise for the reader).
> 
> Of course, I don't think that these issues should delay the current patch, but deserve a ticket of their own to make sure they are tided up.

Attachments (1)

trac_7836-CRT.patch (3.8 KB) - added by cremona 11 years ago.
Applies to 4.3 + patches at #7595

Download all attachments as: .zip

Change History (6)

comment:1 Changed 11 years ago by mhansen

  • Description modified (diff)

Changed 11 years ago by cremona

Applies to 4.3 + patches at #7595

comment:2 Changed 11 years ago by cremona

  • Authors set to John Cremona
  • Cc rlm added
  • Priority changed from major to minor
  • Status changed from new to needs_review

The attached patch is based on 4.3 + the patches at #7595. I tested all files which use either CRT_list or CRT_basis.

comment:3 Changed 11 years ago by rlm

  • Reviewers set to Robert Miller
  • Status changed from needs_review to positive_review

I've tested in sage/rings, and the changes look good to me.

comment:4 Changed 11 years ago by rlm

  • Merged in set to 4.3.1.alpha2
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:5 Changed 11 years ago by mvngu

  • Merged in changed from 4.3.1.alpha2 to sage-4.3.1.alpha2
Note: See TracTickets for help on using tickets.