#5716 closed defect (fixed)
[with patch, positive review] lifting a subdivided matrix should preserve the subdivision, but doesn't.
Reported by: | was | Owned by: | jhpalmieri |
---|---|---|---|
Priority: | major | Milestone: | sage-4.0.1 |
Component: | linear algebra | Keywords: | |
Cc: | jason | Merged in: | 4.0.1.alpha0 |
Authors: | John Palmieri | Reviewers: | Jason Grout |
Report Upstream: | Work issues: | ||
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
sage: a = random_matrix(GF(3),4) sage: a.subdivide(2,2) sage: a [2 0|0 2] [2 1|1 0] [---+---] [1 2|1 0] [1 0|0 1] sage: a.lift() [2 0 0 2] [2 1 1 0] [1 2 1 0] [1 0 0 1]
See also #5717.
Attachments (1)
Change History (11)
comment:1 Changed 12 years ago by
- Description modified (diff)
comment:2 Changed 12 years ago by
- Milestone changed from sage-3.4.1 to sage-3.4.2
- Summary changed from lifting a subdivided matrix should preserve the subdivision, but doesn't. to [duplicate] lifting a subdivided matrix should preserve the subdivision, but doesn't.
comment:3 Changed 12 years ago by
- Summary changed from [duplicate] lifting a subdivided matrix should preserve the subdivision, but doesn't. to lifting a subdivided matrix should preserve the subdivision, but doesn't.
(I don't think this is actually a duplicate.)
comment:4 Changed 12 years ago by
- Owner changed from was to jhpalmieri
- Status changed from new to assigned
- Summary changed from lifting a subdivided matrix should preserve the subdivision, but doesn't. to [with patch, needs review] lifting a subdivided matrix should preserve the subdivision, but doesn't.
Here's a patch. This should change things so that subdivisions are preserved when calling mat.change_ring()
, mat.lift()
, mat.dense_matrix()
, and mat.sparse_matrix()
.
comment:5 Changed 12 years ago by
- Summary changed from [with patch, needs review] lifting a subdivided matrix should preserve the subdivision, but doesn't. to [with patch, needs work] lifting a subdivided matrix should preserve the subdivision, but doesn't.
This applies fine to 4.0.rc1 and all doctests in sage/matrix pass (except the known numerical-noise failure which is nothing to do with this patch). But I'm not completely happy with it, because not all of the functions where the behaviour has changed have doctests to prove it, so I'm changing this to "needs work".
David
comment:6 Changed 12 years ago by
- Summary changed from [with patch, needs work] lifting a subdivided matrix should preserve the subdivision, but doesn't. to [with patch, needs review] lifting a subdivided matrix should preserve the subdivision, but doesn't.
Okay, here's a new patch. I think that this tests everything, although there is at least one function (sparse_matrix, maybe) which is tested in a doctest for another (by looking at a.dense_matrix().sparse_matrix()
or something like that).
Changed 12 years ago by
comment:7 Changed 12 years ago by
- Cc jason added
comment:8 Changed 12 years ago by
- Summary changed from [with patch, needs review] lifting a subdivided matrix should preserve the subdivision, but doesn't. to [with patch, positive review] lifting a subdivided matrix should preserve the subdivision, but doesn't.
It passes doctests (and everything is tested). Looks good to me. Positive review.
comment:9 Changed 12 years ago by
- Resolution set to fixed
- Status changed from assigned to closed
Merged in 4.0.alpha0.
comment:10 Changed 12 years ago by
- Merged in set to 4.0.1.alpha0
- Reviewers set to Jason Grout
Is this a high priority issue? I am not convinced and since there is no patch and no sign of anyone working on fixing this I am bumping this to 3.4.2. There are also various dupes, one of which go reopened, i.e. #5715, so let's figure this out.
Cheers,
Michael