Opened 9 years ago

Last modified 9 years ago

#14958 closed enhancement

Implement pseudo-Conway polynomials — at Version 11

Reported by: pbruin Owned by:
Priority: major Milestone: sage-5.13
Component: finite rings Keywords: Conway polynomial sd51
Cc: roed, jpflori, mraum, fstromberg, JCooley, davidloeffler, dfesti Merged in:
Authors: David Roe, Jean-Pierre Flori Reviewers: Jean-Pierre Flori, Peter Bruin
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #14833, #14957 Stopgaps:

Status badges

Description (last modified by jpflori)

This patch implements pseudo-Conway polynomials, which are used to enable coercion between finite fields. This has been split off from #8335, and is a dependency of that ticket.

Apply:

Change History (13)

comment:1 Changed 9 years ago by pbruin

  • Dependencies changed from #14957 to #14833, #14957

comment:2 Changed 9 years ago by pbruin

  • Cc roed jpflori mraum fstromberg JCooley davidloeffler dfesti added

CC-ing the authors of #8335 and Sage Days 51 participants on this split-off ticket.

comment:3 Changed 9 years ago by pbruin

  • Description modified (diff)
  • Status changed from new to needs_review

comment:4 Changed 9 years ago by pbruin

  • Status changed from needs_review to needs_work

comment:5 Changed 9 years ago by pbruin

  • Description modified (diff)
  • Status changed from needs_work to needs_review

comment:6 Changed 9 years ago by pbruin

I rebased and updated the code written by David Roe and Jean-Pierre Flori.

One important question before me or someone else reviews this: why does this patch introduce objects that are called pseudo-Conway polynomial trees? This strongly suggests that the system of compatible finite fields that it represents forms a tree, which is definitely NOT the case. This is the whole reason why all of this work needs to be done to get such a system of finite fields!

comment:7 Changed 9 years ago by pbruin

The last patch still needed some cleaning up; here is a new one.

Changed 9 years ago by pbruin

implement pseudo-Conway polynomials

comment:8 follow-up: Changed 9 years ago by pbruin

  • Description modified (diff)
  • Status changed from needs_review to needs_info
  • Work issues set to terminology; possible bug

Reasons for status change to needs_info:

comment:9 in reply to: ↑ 8 Changed 9 years ago by jpflori

Replying to pbruin:

Reasons for status change to needs_info:

I agree. I also don't really feel satisfied with the way all these polynomials are stored right now, but I'm not sure I'll come up with anything quickly.

I think it's fixed in David's implementation. That's what got me confused at http://trac.sagemath.org/ticket/8335#comment:55 because I thought the addition David made were not needed, and then i realized they were: http://trac.sagemath.org/ticket/8335#comment:57.

comment:10 Changed 9 years ago by jpflori

I'd say David used the term tree because a PCPT objecgt only contains one layer of references, i.e. a pseudo-conway poly and refs to the PCPTs for n/q where q is a prime divisor of n.

Some minor remarks:

  • I think we should add a warning that PCPTs are only weakrefed. I saw that you correctly took care of that in #8335, thanks for that.
  • At some point the new doc says "The following demonstrate coercions", shouldn't it be "demonstrates"? (please pardon me if I'm wrong, I'm no english expert).
  • The doc changes corresponding to the fix mentionned in http://trac.sagemath.org/ticket/8335#comment:59 are missing; it should not read:
                If False, then `n` is prime and 
                no references are stored (since there is no compatibility 
                condition).
    
    but:
              If ``False``, then `n == 1` and no references are stored 
              (since there is no compatiblity condition).
    
Last edited 9 years ago by jpflori (previous) (diff)

comment:11 Changed 9 years ago by jpflori

  • Description modified (diff)
  • Reviewers set to Jean-Pierre Flori, Peter Bruin
  • Status changed from needs_info to needs_review
  • Work issues terminology; possible bug deleted

The last patch renames some classes, methods and functions.

It also fixes doctests and documentation.

Patchbot, apply:
* trac_14958-pseudo_conway.patch
* trac_14958-rename-doctests.patch

Changed 9 years ago by jpflori

Note: See TracTickets for help on using tickets.