Opened 9 years ago

Closed 9 years ago

#15626 closed enhancement (fixed)

Further improvements to splitting_field()

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

Status badges

Description (last modified by Jeroen Demeyer)

  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 9 years ago by Jeroen Demeyer

Description: modified (diff)

comment:2 Changed 9 years ago by Jeroen Demeyer

Branch: u/jdemeyer/ticket/15626
Created: Jan 3, 2014, 12:30:16 PMJan 3, 2014, 12:30:16 PM
Modified: Jan 3, 2014, 5:05:53 PMJan 3, 2014, 5:05:53 PM

comment:3 Changed 9 years ago by Jeroen Demeyer

Commit: 8f0baae60ddc1d55f39b45ee1fbd6d9518e55cb4
Dependencies: #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 9 years ago by git

Commit: 8f0baae60ddc1d55f39b45ee1fbd6d9518e55cb4dd4fe5f72f191043abd9919e26c77666af22d589

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 9 years ago by Jeroen Demeyer

Dependencies: #2217
Modified: Jan 3, 2014, 7:49:59 PMJan 3, 2014, 7:49:59 PM

comment:6 Changed 9 years ago by Jeroen Demeyer

Dependencies: #2217

comment:7 Changed 9 years ago by git

Commit: dd4fe5f72f191043abd9919e26c77666af22d589df52508379069aa8fb24c6620e11c5769dc6af13

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 9 years ago by git

Commit: df52508379069aa8fb24c6620e11c5769dc6af13776795d8eae9b2d4751fdb5968bf4c3a06539914

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

776795dDo polynomial consistency check only for minimal dm

comment:9 Changed 9 years ago by Jeroen Demeyer

Description: modified (diff)
Status: newneeds_review

comment:10 Changed 9 years ago by John 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 9 years ago by John Cremona

Reviewers: John Cremona
Status: needs_reviewpositive_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 9 years ago by Volker Braun

Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.