Opened 7 years ago

Closed 6 years ago

#16130 closed defect (fixed)

Matrices over Universal Cyclotomic field failing to echelonize/inverse

Reported by: jipilab Owned by:
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: number fields Keywords: cyclotomic field, matrix, echelon form, inverse, days57
Cc: sage-combinat, nthiery, stumpc5, vripoll Merged in:
Authors: Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

Here is an instance of smallest failing example.

    sage: UCF.<E>=UniversalCyclotomicField()
    sage: MS=MatrixSpace(UCF,2,2)
    sage: bad_mat=MS([-4, 2*E(7)^6, -5*E(13)^3 + 5*E(13)^8 - 4*E(13)^9, 0]) #this matrix was obtained via random_element
    sage: bad_mat._echelon_classical()
    Traceback (most recent call last)
    [...]
    OverflowError: value too large to convert to int
    sage: bad_mat.inverse()
    Traceback (most recent call last)
    [...]
    NotImplementedError: value too large to convert to int
    Echelon form not implemented over 'Universal Cyclotomic Field'.

The error handling of inverse should be looked at to reflect the origin of the error. In this case, that the echelonize method converts to integers and therefore fails for too big denominators but is still implemented over Universal Cyclotomic Field. For example, the following case does work:

    sage: good_mat=MS([E(4), 0, -2*E(17)^2 - 4*E(17)^3, -2])
    sage: good_mat.echelon_form()
    [1 0]
    [0 1]
    sage: good_mat.inverse()
    [                -E(4)                     0]
    [E(68)^25 + 2*E(68)^29                  -1/2]

Change History (9)

comment:1 Changed 7 years ago by jipilab

  • Component changed from PLEASE CHANGE to number fields
  • Type changed from PLEASE CHANGE to defect

comment:2 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:3 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:4 Changed 6 years ago by vdelecroix

Hello,

In #18152, I reimplemented the universal cyclotomic field using libgap (faster and more reliable). In that version the issue disappears. My goal would be to close this ticket as "won't fix" as soon as #18152 would be reviewed.

Vincent

comment:5 Changed 6 years ago by jipilab

Hi Vincent,

This is great! Looking forward to it!

Jean-Philippe

comment:6 Changed 6 years ago by vdelecroix

Hi Jean-Philippe,

It can be even faster if you review it!

Vincent

comment:7 Changed 6 years ago by chapoton

  • Milestone changed from sage-6.4 to sage-duplicate/invalid/wontfix
  • Status changed from new to needs_review

Now works with libgap version

comment:8 Changed 6 years ago by mmezzarobba

  • Status changed from needs_review to positive_review

comment:9 Changed 6 years ago by vbraun

  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.