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:  sage6.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: 
Description (last modified by )
 Implement an "early abort" option to stop if it's clear that the degree will be huge.
 Add some more examples (more indirect examples will be added by #11905)
Change History (12)
comment:1 Changed 9 years ago by
Description:  modified (diff) 

comment:2 Changed 9 years ago by
Branch:  → u/jdemeyer/ticket/15626 

Created:  Jan 3, 2014, 12:30:16 PM → Jan 3, 2014, 12:30:16 PM 
Modified:  Jan 3, 2014, 5:05:53 PM → Jan 3, 2014, 5:05:53 PM 
comment:3 Changed 9 years ago by
Commit:  → 8f0baae60ddc1d55f39b45ee1fbd6d9518e55cb4 

Dependencies:  → #2217 
comment:4 Changed 9 years ago by
Commit:  8f0baae60ddc1d55f39b45ee1fbd6d9518e55cb4 → dd4fe5f72f191043abd9919e26c77666af22d589 

Branch pushed to git repo; I updated commit sha1. This was a forced push. Recent commits:
dd4fe5f  Further improvements to splitting_field()

c50eb3e  No need to specify caller_name in verbose()

6df69b6  Add comments, small cosmetic changes

9e2dd44  Make q monic before computing cubic resolvent

9b5558e  Implement splitting fields for number fields

comment:5 Changed 9 years ago by
Dependencies:  #2217 

Modified:  Jan 3, 2014, 7:49:59 PM → Jan 3, 2014, 7:49:59 PM 
comment:6 Changed 9 years ago by
Dependencies:  → #2217 

comment:7 Changed 9 years ago by
Commit:  dd4fe5f72f191043abd9919e26c77666af22d589 → df52508379069aa8fb24c6620e11c5769dc6af13 

Branch pushed to git repo; I updated commit sha1. This was a forced push. Recent commits:
df52508  Further improvements to splitting_field()

c50eb3e  No need to specify caller_name in verbose()

6df69b6  Add comments, small cosmetic changes

9e2dd44  Make q monic before computing cubic resolvent

9b5558e  Implement splitting fields for number fields

comment:8 Changed 9 years ago by
Commit:  df52508379069aa8fb24c6620e11c5769dc6af13 → 776795d8eae9b2d4751fdb5968bf4c3a06539914 

Branch pushed to git repo; I updated commit sha1. New commits:
776795d  Do polynomial consistency check only for minimal dm

comment:9 Changed 9 years ago by
Description:  modified (diff) 

Status:  new → needs_review 
comment:10 Changed 9 years ago by
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
Reviewers:  → John Cremona 

Status:  needs_review → 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 9 years ago by
Resolution:  → fixed 

Status:  positive_review → closed 
New commits:
Further improvements to splitting_field()
No need to specify caller_name in verbose()
Add comments, small cosmetic changes
Make q monic before computing cubic resolvent
Implement splitting fields for number fields