Opened 10 years ago

Closed 9 years ago

# pointless computations when converting number field elements

Reported by: Owned by: mstreng davidloeffler major sage-5.12 number fields sd51 jdemeyer, daniels, JCooley sage-5.12.beta0 Jenny Cooley David Loeffler N/A u/davidloeffler/ticket/12270

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.

### comment:1 follow-up: ↓ 2 Changed 10 years ago by jdemeyer

I'm a but confused. Exactly where and when are roots unneccessarily computed?

### comment:2 in reply to: ↑ 1 ; follow-up: ↓ 3 Changed 10 years ago by mstreng

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 10 years ago by jdemeyer

• Description modified (diff)

Thanks for the clarification. I agree.

### comment:4 Changed 10 years ago by mstreng

• Description modified (diff)

### comment:7 Changed 9 years ago by JCooley

• Cc JCooley added; yiweishe removed

### Changed 9 years ago by JCooley

swaps the order of the two processes

### Changed 9 years ago by JCooley

fixes an error that came up in testing

### comment:8 Changed 9 years ago by JCooley

Sorry, newbie error, I did hg_commit() before I had tested the patch, hence there being two patches.

### comment:9 Changed 9 years ago by JCooley

• Status changed from new to needs_review

### comment:10 Changed 9 years ago by davidloeffler

• Branch set to u/davidloeffler/ticket/12270

### comment:11 Changed 9 years ago by davidloeffler

• Authors set to Jenny Cooley
• 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 9 years ago by jdemeyer

• Milestone changed from sage-5.11 to sage-5.12

### comment:13 Changed 9 years ago by jdemeyer

• Merged in set to sage-5.12.beta0
• Resolution set to fixed
• Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.