Opened 7 years ago
Closed 6 years ago
#12270 closed enhancement (fixed)
pointless computations when converting number field elements
Reported by: | mstreng | Owned by: | davidloeffler |
---|---|---|---|
Priority: | major | Milestone: | sage-5.12 |
Component: | number fields | Keywords: | sd51 |
Cc: | jdemeyer, daniels, JCooley | Merged in: | sage-5.12.beta0 |
Authors: | Jenny Cooley | Reviewers: | David Loeffler |
Report Upstream: | N/A | Work issues: | |
Branch: | u/davidloeffler/ticket/12270 | Commit: | |
Dependencies: | Stopgaps: |
Description (last modified by )
In /sage/rings/number_field/number_field.py
(as per #11869), when converting number field elements, the parts
- List of candidates for K(x)
- Find a common field F into which KF and LF both embed
are independent. The first can be very slow, while the second is likely to fail. So I propose to swap these two parts. See #12269 for an example and for indicator code.
Attachments (2)
Change History (15)
comment:1 follow-up: ↓ 2 Changed 7 years ago by
comment:2 in reply to: ↑ 1 ; follow-up: ↓ 3 Changed 7 years ago by
Replying to jdemeyer:
I'm a but confused. Exactly where and when are roots unneccessarily computed?
In /sage/rings/number_field/number_field.py
(as per #11869) the parts
- "# List of candidates for K(x)" and
- "# Find a common field F into which KF and LF both embed."
are independent. The first can be very slow, while the second is likely to fail. So I propose to swap these two parts.
comment:3 in reply to: ↑ 2 Changed 7 years ago by
- Description modified (diff)
Thanks for the clarification. I agree.
comment:4 Changed 7 years ago by
- Description modified (diff)
comment:5 Changed 7 years ago by
- Cc daniels added
comment:6 Changed 6 years ago by
- Cc yiweishe added
comment:7 Changed 6 years ago by
- Cc JCooley added; yiweishe removed
- Keywords sd51 added
comment:8 Changed 6 years ago by
Sorry, newbie error, I did hg_commit() before I had tested the patch, hence there being two patches.
comment:9 Changed 6 years ago by
- Status changed from new to needs_review
comment:10 Changed 6 years ago by
- Branch set to u/davidloeffler/ticket/12270
comment:11 Changed 6 years ago by
- Reviewers set to David Loeffler
- Status changed from needs_review to positive_review
Does what it says on the tin. Positive review.
comment:12 Changed 6 years ago by
- Milestone changed from sage-5.11 to sage-5.12
comment:13 Changed 6 years ago by
- Merged in set to sage-5.12.beta0
- Resolution set to fixed
- Status changed from positive_review to closed
I'm a but confused. Exactly where and when are roots unneccessarily computed?