Opened 3 years ago

Closed 3 years ago

Last modified 2 years ago

#28398 closed enhancement (fixed)

_richcmp_ for quaternion algebra elements

Reported by: chapoton Owned by: chapoton
Priority: major Milestone: sage-8.9
Component: algebra Keywords:
Cc: tscrim, chapoton Merged in:
Authors: Frédéric Chapoton Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: c8425cb (Commits, GitHub, GitLab) Commit:
Dependencies: Stopgaps:

Status badges

Description

instead of the old-style _cmp_

Change History (8)

comment:1 Changed 3 years ago by chapoton

  • Branch set to u/chapoton/28398
  • Commit set to c8425cb310b3133b0b51bbdb286fd7847f0d025c
  • Status changed from new to needs_review

New commits:

​c8425cbrichcmp for quaternion algebras elements

comment:2 Changed 3 years ago by chapoton

  • Cc tscrim added

bot is morally green, please review

comment:3 Changed 3 years ago by tscrim

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

LGTM.

comment:4 Changed 3 years ago by vbraun

  • Branch changed from u/chapoton/28398 to c8425cb310b3133b0b51bbdb286fd7847f0d025c
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:5 Changed 2 years ago by mmasdeu

  • Commit c8425cb310b3133b0b51bbdb286fd7847f0d025c deleted
  • Owner changed from (none) to tscrim

This breaks quaternion algebra comparisons quite badly: 

sage: B.<i,j,k> = QuaternionAlgebra(6)
sage: B(1) == B(-1)
False
sage: B(1) != B(-1)
True
Version 0, edited 2 years ago by mmasdeu (next)

comment:6 Changed 2 years ago by mmasdeu

  • Cc chapoton added

comment:7 Changed 2 years ago by mmasdeu

  • Owner changed from tscrim to chapoton

comment:8 Changed 2 years ago by tscrim

So this change did not cause that problem, but instead #28595. The problem comes from the fact that mpz_cmp yields arbitrary positive/negative numbers, but rich_to_bool assumes the input is in [-1, 0, 1]. Instead, we just need to use rich_to_bool_sgn. This is now #29094.

Last edited 2 years ago by tscrim (previous) (diff)
Note: See TracTickets for help on using tickets.