Opened 11 years ago

Closed 10 years ago

Last modified 10 years ago

#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 was)

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)

trac_5716.patch (9.3 KB) - added by jhpalmieri 10 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 11 years ago by was

  • Description modified (diff)

comment:2 Changed 11 years ago by mabshoff

  • 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.

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

comment:3 Changed 11 years ago by jhpalmieri

  • 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 11 years ago by jhpalmieri

  • 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 10 years ago by davidloeffler

  • 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 10 years ago by jhpalmieri

  • 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 10 years ago by jhpalmieri

comment:7 Changed 10 years ago by jason

  • Cc jason added

comment:8 Changed 10 years ago by jason

  • 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 10 years ago by mhansen

  • Resolution set to fixed
  • Status changed from assigned to closed

Merged in 4.0.alpha0.

comment:10 Changed 10 years ago by mhansen

  • Authors set to John Palmieri
  • Merged in set to 4.0.1.alpha0
  • Reviewers set to Jason Grout
Note: See TracTickets for help on using tickets.