Opened 3 years ago

Closed 3 years ago

#21596 closed defect (fixed)

Fix so that Matrix charpoly(algorithm='flint') doesn't destroy the polynomial ring generator

Reported by: bober Owned by:
Priority: major Milestone: sage-7.4
Component: linear algebra Keywords:
Cc: Merged in:
Authors: Jonathan Bober Reviewers: Marc Mezzarobba
Report Upstream: N/A Work issues:
Branch: bb7f508 (Commits) Commit: bb7f5087cf59c03672b4ea188ef2c1d5aa1affff
Dependencies: Stopgaps:

Description

One line fix coming shortly, I hope.

Change History (5)

comment:1 Changed 3 years ago by bober

I'm hoping to learn how to be a better "sage citizen" who can at least easily contribute small patches, but in case that doesn't happen in the next few days (or months, or years), I ought to actually record the fix:

in src/sage/matrix/matrix_integer_dense.pyx

the lines

        if algorithm == 'flint' or (algorithm == 'linbox' and not USE_LINBOX_POLY):
            g = PolynomialRing(ZZ,names = var).gen()
            sig_on()
            fmpz_mat_charpoly(g.__poly,self._matrix)
            sig_off()

should be something like

        if algorithm == 'flint' or (algorithm == 'linbox' and not USE_LINBOX_POLY):
            g = (<Polynomial_integer_dense_flint>(PolynomialRing(ZZ,names = var).gen()))._new()
            sig_on()
            fmpz_mat_charpoly(g.__poly,self._matrix)
            sig_off()

(There's also a misleading line in the docstring above which says that linbox isn't used for this on 64-bit systems.)

comment:2 Changed 3 years ago by bober

  • Summary changed from Matrix charpoly(algorithm='flint') destroys the polynomial ring generator to Fix so that Matrix charpoly(algorithm='flint') doesn't destroy the polynomial ring generator

comment:3 Changed 3 years ago by bober

  • Authors set to Jonathan Bober
  • Branch set to u/bober/fix_so_that_matrix_charpoly_algorithm__flint___doesn_t_destroy_the_polynomial_ring_generator
  • Commit set to bb7f5087cf59c03672b4ea188ef2c1d5aa1affff
  • Status changed from new to needs_review

comment:4 Changed 3 years ago by mmezzarobba

  • Reviewers set to Marc Mezzarobba
  • Status changed from needs_review to positive_review

Looks good to me; thanks for the fix!

comment:5 Changed 3 years ago by vbraun

  • Branch changed from u/bober/fix_so_that_matrix_charpoly_algorithm__flint___doesn_t_destroy_the_polynomial_ring_generator to bb7f5087cf59c03672b4ea188ef2c1d5aa1affff
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.