Opened 2 months ago

Closed 8 weeks ago

Last modified 3 weeks ago

#26872 closed defect (fixed)

Implement abs and sqrt for UniversalCyclotomicField elements

Reported by: slelievre Owned by:
Priority: major Milestone: sage-8.6
Component: number fields Keywords: UniversalCyclotomicField, universal cyclotomic field
Cc: slelievre, vdelecroix, tscrim Merged in:
Authors: Frédéric Chapoton Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: db44a9e (Commits) Commit:
Dependencies: #26875 Stopgaps:

Description (last modified by slelievre)

Inspired by this report by Ask Sage user Assombrance:

For abs, we need to implement __abs__ for elements of the universal cyclotomic field; it could be computed as the square root of z * z.conjugate() or of z.real()**2 + z.imag()**2, or by using to_cyclotomic_field, or by changing ring to QQbar.

Change History (8)

comment:1 Changed 2 months ago by slelievre

  • Cc slelievre vdelecroix added
  • Description modified (diff)
  • Keywords universal cyclotomic field added
  • Summary changed from Implement abs for UniversalCyclotomicField elements to Implement abs and sqrt for UniversalCyclotomicField elements

comment:2 Changed 2 months ago by chapoton

  • Branch set to u/chapoton/26872
  • Commit set to db44a9e75ce0ae0b9c4ea55720fadfdc8822d112
  • Status changed from new to needs_info

New commits:

b7f0b12new method "is_integral" for elements of universal cyclotomic field
db44a9etrac 26872 adding abs and sqrt to universal cyclotomic field

comment:3 Changed 2 months ago by chapoton

  • Dependencies set to #26875

comment:4 Changed 2 months ago by chapoton

  • Authors set to Frédéric Chapoton
  • Cc tscrim added
  • Status changed from needs_info to needs_review

green bot, so this seems to be ready for review

comment:5 Changed 2 months ago by tscrim

  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_review to positive_review

This will work well as a first implementation. If we need to optimize either of these operations later on, then we can do it then.

comment:6 Changed 8 weeks ago by vbraun

  • Branch changed from u/chapoton/26872 to db44a9e75ce0ae0b9c4ea55720fadfdc8822d112
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:7 Changed 8 weeks ago by embray

  • Commit db44a9e75ce0ae0b9c4ea55720fadfdc8822d112 deleted
  • Milestone changed from sage-8.5 to sage-8.6

comment:8 Changed 3 weeks ago by slelievre

Oops. The method should have been called __abs__ rather than abs.

Follow-up at #27201.

Note: See TracTickets for help on using tickets.