Opened 11 years ago
Closed 7 years ago
#10843 closed enhancement (fixed)
Faster listing of number field homsets
Reported by:  fwclarke  Owned by:  davidloeffler 

Priority:  minor  Milestone:  sage6.4 
Component:  number fields  Keywords:  
Cc:  lftabera, robharron, tfeulner, SimonKing  Merged in:  
Authors:  Francis Clarke  Reviewers:  Jeroen Demeyer 
Report Upstream:  N/A  Work issues:  
Branch:  e2ee8a3 (Commits, GitHub, GitLab)  Commit:  e2ee8a3231962e830de11b1fc94598c61fe0c9c1 
Dependencies:  Stopgaps: 
Description
The patch speeds up the calculation of homomorphisms between number fields. This is mainly achieved by avoiding unnecessary checking. Doctests are also updated.
Attachments (1)
Change History (16)
comment:1 Changed 11 years ago by
 Status changed from new to needs_review
comment:2 Changed 10 years ago by
 Milestone set to sage5.1
Changed 9 years ago by
comment:3 Changed 9 years ago by
 Cc lftabera robharron tfeulner added
The patch has been rebased for Sage5.9.
comment:4 Changed 9 years ago by
 Cc SimonKing added
comment:5 Changed 8 years ago by
 Milestone changed from sage5.11 to sage5.12
comment:6 Changed 8 years ago by
 Milestone changed from sage6.1 to sage6.2
comment:7 Changed 8 years ago by
 Milestone changed from sage6.2 to sage6.3
comment:8 Changed 8 years ago by
 Branch set to public/10843
 Commit set to 51569d96fee90505298606630dc25c87f5d43edf
Hi,
Looks nice.
I uploaded your patch in a git branch and added three commits above:
 the first one to remove the trailing whitespaces
 the second one to fits with Sage standard for the docs and make the code more compatible with Python 3
 the third to have a better caching
Please, tell me what you think.
Vincent
New commits:
20f21a6  #10843 faster lising of number field homsets

4040bfd  trac #10843: remove trailing whitespaces

a2abefa  trac #10843: documentation, indentation

51569d9  trac #10843: improved caching

comment:9 Changed 8 years ago by
Thanks for this. It all seems to be working perfectly.
comment:10 Changed 7 years ago by
 Milestone changed from sage6.3 to sage6.4
comment:11 followup: ↓ 13 Changed 7 years ago by
 Status changed from needs_review to needs_work
Why the separate branches for D
absolute/relative? The following works in both cases:
roots = D.polynomial().roots(ring=C, multiplicities=False)
comment:12 Changed 7 years ago by
 Commit changed from 51569d96fee90505298606630dc25c87f5d43edf to e2ee8a3231962e830de11b1fc94598c61fe0c9c1
comment:13 in reply to: ↑ 11 Changed 7 years ago by
 Status changed from needs_work to needs_review
Replying to jdemeyer:
Why the separate branches for
D
absolute/relative?
Assuming that you meant C
rather than D
, I've changed the code as suggested. I don't know why this was done separately. Perhaps the simpler approach didn't work when this ticket began four years ago.
I've made one other change which should have been made before. It solves a problem which arose in http://ask.sagemath.org/question/24173/homomorphismsforrelativenumberfields/.
comment:14 Changed 7 years ago by
 Reviewers set to Jeroen Demeyer
 Status changed from needs_review to positive_review
comment:15 Changed 7 years ago by
 Branch changed from public/10843 to e2ee8a3231962e830de11b1fc94598c61fe0c9c1
 Resolution set to fixed
 Status changed from positive_review to closed
rebased for 5.9