Opened 12 years ago
Closed 5 weeks ago
#7989 closed defect (fixed)
Minpoly doesn't work for all matrices
Reported by: | jason | Owned by: | was |
---|---|---|---|
Priority: | major | Milestone: | sage-9.7 |
Component: | linear algebra | Keywords: | |
Cc: | kcrisman, rbeezer, mhansen, hedtke, tscrim | Merged in: | |
Authors: | Jason Grout, Frédéric Chapoton | Reviewers: | Travis Scrimshaw |
Report Upstream: | N/A | Work issues: | |
Branch: | 329d472 (Commits, GitHub, GitLab) | Commit: | 329d4724ceb242e197af9a7a5aa4693d75a2308a |
Dependencies: | Stopgaps: |
Description (last modified by )
Right now, not all matrices can compute minpolys. This patch exposes these matrices.
Attachments (1)
Change History (24)
comment:1 Changed 12 years ago by
- Description modified (diff)
Changed 12 years ago by
comment:2 Changed 12 years ago by
comment:3 Changed 12 years ago by
- Cc mhansen added
comment:4 Changed 12 years ago by
- Status changed from new to needs_work
comment:5 Changed 11 years ago by
- Cc hedtke added
Sorry, I don't understand the reason for this function. What is the idea behind it?
comment:6 Changed 11 years ago by
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 11 years ago by
OK. Works for me, too. Is it favored that we include a test that purposely fails?
comment:8 in reply to: ↑ 7 Changed 11 years ago by
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.
comment:9 Changed 9 years ago by
- Milestone changed from sage-5.11 to sage-5.12
comment:10 Changed 8 years ago by
- Milestone changed from sage-6.1 to sage-6.2
comment:11 Changed 8 years ago by
sage-6.2.beta4:
sage -t src/sage/matrix/matrix_generic_dense.pyx # 1 doctest failed sage -t src/sage/matrix/matrix_dense.pyx # 1 doctest failed
comment:12 Changed 8 years ago by
- Milestone changed from sage-6.2 to sage-6.3
comment:13 Changed 8 years ago by
- Milestone changed from sage-6.3 to sage-6.4
comment:14 Changed 7 years ago by
- Branch set to u/jdemeyer/minpoly_doesn_t_work_for_all_matrices
comment:15 Changed 6 weeks ago by
- Branch changed from u/jdemeyer/minpoly_doesn_t_work_for_all_matrices to public/ticket/7989
- Commit set to 08701a4701f1acad04a8d9663f103540c71a7cc4
New commits:
08701a4 | test that minpoly works
|
comment:16 Changed 6 weeks ago by
- Commit changed from 08701a4701f1acad04a8d9663f103540c71a7cc4 to ce364f737b21c2131710bbfd199936b520dc2198
Branch pushed to git repo; I updated commit sha1. New commits:
ce364f7 | only test minpoly for exact rings
|
comment:17 Changed 6 weeks ago by
- Commit changed from ce364f737b21c2131710bbfd199936b520dc2198 to 216d945af1a918c569191473cb1890733769318d
Branch pushed to git repo; I updated commit sha1. New commits:
216d945 | use Gap for minpoly of Gap matrices
|
comment:18 Changed 6 weeks ago by
- Commit changed from 216d945af1a918c569191473cb1890733769318d to 9f476b44608a2acacf7e86bfec99a754fa80f796
Branch pushed to git repo; I updated commit sha1. New commits:
9f476b4 | add doc, skip test
|
comment:19 Changed 6 weeks ago by
- Commit changed from 9f476b44608a2acacf7e86bfec99a754fa80f796 to 329d4724ceb242e197af9a7a5aa4693d75a2308a
Branch pushed to git repo; I updated commit sha1. New commits:
329d472 | skip another test
|
comment:20 Changed 6 weeks ago by
- Milestone changed from sage-6.4 to sage-9.7
- Status changed from needs_work to needs_review
comment:22 Changed 6 weeks ago by
- Description modified (diff)
- Reviewers set to Travis Scrimshaw
- Status changed from needs_review to positive_review
LGTM.
comment:23 Changed 5 weeks ago by
- Branch changed from public/ticket/7989 to 329d4724ceb242e197af9a7a5aa4693d75a2308a
- Resolution set to fixed
- Status changed from positive_review to closed
Here are the bugs (doctests that fail) this patch exposes: