Ticket #7836 (closed defect: fixed)

Opened 3 years ago

Last modified 3 years ago

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 Work issues:
Report Upstream: N/A Reviewers: Robert Miller
Authors: John Cremona Merged in: sage-4.3.1.alpha2
Dependencies: Stopgaps:

Description (last modified by mhansen) (diff)

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

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

Change History

comment:1 Changed 3 years ago by mhansen

  • Description modified (diff)

Changed 3 years ago by cremona

Applies to 4.3 + patches at #7595

comment:2 Changed 3 years ago by cremona

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

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 3 years ago by rlm

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

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

comment:4 Changed 3 years ago by rlm

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

comment:5 Changed 3 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.