Opened 7 years ago
Closed 7 years ago
#15626 closed enhancement (fixed)
Further improvements to splitting_field()
Reported by:  jdemeyer  Owned by:  

Priority:  major  Milestone:  sage6.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 )
 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 7 years ago by
 Description modified (diff)
comment:2 Changed 7 years ago by
 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 7 years ago by
 Commit set to 8f0baae60ddc1d55f39b45ee1fbd6d9518e55cb4
 Dependencies set to #2217
comment:4 Changed 7 years ago by
 Commit changed from 8f0baae60ddc1d55f39b45ee1fbd6d9518e55cb4 to 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 7 years ago by
 Dependencies #2217 deleted
 Modified changed from 01/03/14 19:49:59 to 01/03/14 19:49:59
comment:6 Changed 7 years ago by
 Dependencies set to #2217
comment:7 Changed 7 years ago by
 Commit changed from dd4fe5f72f191043abd9919e26c77666af22d589 to 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 7 years ago by
 Commit changed from df52508379069aa8fb24c6620e11c5769dc6af13 to 776795d8eae9b2d4751fdb5968bf4c3a06539914
Branch pushed to git repo; I updated commit sha1. New commits:
776795d  Do polynomial consistency check only for minimal dm

comment:9 Changed 7 years ago by
 Description modified (diff)
 Status changed from new to needs_review
comment:10 Changed 7 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 7 years ago by
 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 7 years ago by
 Resolution set to fixed
 Status changed from positive_review to 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