Ticket #7989 (needs_work defect)

Opened 3 years ago

Last modified 23 months ago

Minpoly doesn't work for all matrices

Reported by: jason Owned by: was
Priority: major Milestone: sage-5.10
Component: linear algebra Keywords:
Cc: kcrisman, rbeezer, mhansen, hedtke Work issues:
Report Upstream: N/A Reviewers:
Authors: Jason Grout Merged in:
Dependencies: Stopgaps:

Description (last modified by jason) (diff)

Right now, not all matrices can compute minpolys. This patch exposes these matrices.

Depends on #6936

Attachments

trac-7989-minpoly-test.patch Download (1.1 KB) - added by jason 3 years ago.

Change History

comment:1 Changed 3 years ago by jason

  • Description modified (diff)

Changed 3 years ago by jason

comment:2 Changed 3 years ago by jason

Here are the bugs (doctests that fail) this patch exposes:

	sage -t  "4.3.1.rc0/devel/sage-main/sage/matrix/matrix1.pyx"
	sage -t  "4.3.1.rc0/devel/sage-main/sage/matrix/matrix_dense.pyx"
	sage -t  "4.3.1.rc0/devel/sage-main/sage/matrix/matrix_generic_dense.pyx"
	sage -t  "4.3.1.rc0/devel/sage-main/sage/matrix/matrix_integer_2x2.pyx"

comment:3 Changed 3 years ago by jason

  • Cc mhansen added
  • Authors set to Jason Grout

comment:4 Changed 3 years ago by jason

  • Status changed from new to needs_work

comment:5 Changed 23 months ago by hedtke

  • Cc hedtke added

Sorry, I don't understand the reason for this function. What is the idea behind it?

comment:6 Changed 23 months ago by was

This is a great patch. I can't believe this has got ignored for the last 1.5 years.

Regarding what needs to be fixed, the test failure you claim for matrix_integer_2x2 is:

deep:sage wstein$ sage -t matrix/matrix_integer_2x2.pyx
sage -t  "matrix/matrix_integer_2x2.pyx"                    
**********************************************************************
File "/Users/wstein/sage/install/sage-4.7.1.rc0/devel/sage-main/sage/matrix/matrix_integer_2x2.pyx", line 101:
    sage: TestSuite(m).run()
Expected nothing
Got:
    Failure in _test_minpoly:
    Traceback (most recent call last):
      File "/Users/wstein/sage/install/current/local/lib/python/site-packages/sage/misc/sage_unittest.py", line 275, in run
        test_method(tester = tester)
      File "matrix2.pyx", line 1302, in sage.matrix.matrix2.Matrix._test_minpoly (sage/matrix/matrix2.c:8933)
      File "polynomial_element.pyx", line 358, in sage.rings.polynomial.polynomial_element.Polynomial.subs (sage/rings/polynomial/polynomial_element.c:5624)
      File "polynomial_element.pyx", line 557, in sage.rings.polynomial.polynomial_element.Polynomial.__call__ (sage/rings/polynomial/polynomial_element.c:5819)
      File "polynomial_element.pyx", line 638, in sage.rings.polynomial.polynomial_element.Polynomial.__call__ (sage/rings/polynomial/polynomial_element.c:7244)
      File "element.pyx", line 1302, in sage.structure.element.RingElement.__add__ (sage/structure/element.c:11504)
      File "coerce.pyx", line 766, in sage.structure.coerce.CoercionModel_cache_maps.bin_op (sage/structure/coerce.c:7337)
    TypeError: unsupported operand parent(s) for '+': 'Space of 2x2 integer matrices' and 'Integer Ring'
    ------------------------------------------------------------
    The following tests failed: _test_minpoly
**********************************************************************
1 items had failures:
   1 of   6 in __main__.example_7
***Test Failed*** 1 failures.
For whitespace errors, see the file /Users/wstein/.sage//tmp/.doctest_matrix_integer_2x2.py
	 [4.6 s]

This is because substitution isn't even implemented for that class:

sage: MS = sage.matrix.matrix_integer_2x2.MatrixSpace_ZZ_2x2()
sage: a = MS([1,2,3,4])
sage: a.minpoly()
x^2 - 5*x - 2
sage: a.minpoly()(a)
---------------------------------------------------------------------------
TypeError     

Also, if one adds this for minpoly, it would make sense to also add _test_charpoly.

-- William

comment:7 follow-up: ↓ 8 Changed 23 months ago by hedtke

OK. Works for me, too. Is it favored that we include a test that purposely fails?

comment:8 in reply to: ↑ 7 Changed 23 months ago by rbeezer

Replying to hedtke:

OK. Works for me, too. Is it favored that we include a test that purposely fails?

I think the idea is to include the test-suite checking and add the fixes needed to make the tests pass.

Note: See TracTickets for help on using tickets.