Opened 6 years ago

Closed 6 years ago

#15626 closed enhancement (fixed)

Further improvements to splitting_field()

Reported by: jdemeyer Owned by:
Priority: major Milestone: sage-6.1
Component: number fields Keywords:
Cc: cremona, chapoton Merged in:
Authors: Jeroen Demeyer Reviewers: John Cremona
Report Upstream: N/A Work issues:
Branch: u/jdemeyer/ticket/15626 (Commits) Commit: 776795d8eae9b2d4751fdb5968bf4c3a06539914
Dependencies: #2217 Stopgaps:

Description (last modified by jdemeyer)

  1. Implement an "early abort" option to stop if it's clear that the degree will be huge.
  2. Add some more examples (more indirect examples will be added by #11905)

Change History (12)

comment:1 Changed 6 years ago by jdemeyer

  • Description modified (diff)

comment:2 Changed 6 years ago by jdemeyer

  • Branch set to u/jdemeyer/ticket/15626
  • Created changed from 01/03/14 12:30:16 to 01/03/14 12:30:16
  • Modified changed from 01/03/14 17:05:53 to 01/03/14 17:05:53

comment:3 Changed 6 years ago by jdemeyer

  • Commit set to 8f0baae60ddc1d55f39b45ee1fbd6d9518e55cb4
  • Dependencies set to #2217

New commits:

8f0baaeFurther improvements to splitting_field()
c50eb3eNo need to specify caller_name in verbose()
6df69b6Add comments, small cosmetic changes
9e2dd44Make q monic before computing cubic resolvent
9b5558eImplement splitting fields for number fields

comment:4 Changed 6 years ago by git

  • Commit changed from 8f0baae60ddc1d55f39b45ee1fbd6d9518e55cb4 to dd4fe5f72f191043abd9919e26c77666af22d589

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

dd4fe5fFurther improvements to splitting_field()
c50eb3eNo need to specify caller_name in verbose()
6df69b6Add comments, small cosmetic changes
9e2dd44Make q monic before computing cubic resolvent
9b5558eImplement splitting fields for number fields

comment:5 Changed 6 years ago by jdemeyer

  • Dependencies #2217 deleted
  • Modified changed from 01/03/14 19:49:59 to 01/03/14 19:49:59

comment:6 Changed 6 years ago by jdemeyer

  • Dependencies set to #2217

comment:7 Changed 6 years ago by git

  • Commit changed from dd4fe5f72f191043abd9919e26c77666af22d589 to df52508379069aa8fb24c6620e11c5769dc6af13

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

df52508Further improvements to splitting_field()
c50eb3eNo need to specify caller_name in verbose()
6df69b6Add comments, small cosmetic changes
9e2dd44Make q monic before computing cubic resolvent
9b5558eImplement splitting fields for number fields

comment:8 Changed 6 years ago by git

  • Commit changed from df52508379069aa8fb24c6620e11c5769dc6af13 to 776795d8eae9b2d4751fdb5968bf4c3a06539914

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

776795dDo polynomial consistency check only for minimal dm

comment:9 Changed 6 years ago by jdemeyer

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

comment:10 Changed 6 years ago by cremona

Note to self as reviewer: commits up to c50eb3e have been merged in #2217, so the ones to review here are: df52508, 776795d.

comment:11 Changed 6 years ago by cremona

  • Reviewers set to John Cremona
  • Status changed from needs_review to positive_review

The Abort class is a great idea and very well implemented. It would never have occurred to me to implement the early abort this way, and it has led me to think about other situations where a similar strategy might be useful! The examples are good.

All tests in sage/rings pass. For some reason when I tested the whole of Sage built with this branch I got various doctest errors in sage/crypto but I cannot believe that it has anything to do with these changes, so they are not stopping me giving it a positive review. Now I am looking forward to looking at the elliptic curve division field code!

comment:12 Changed 6 years ago by vbraun

  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.