Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#5715 closed defect (fixed)

[with patch; positive review] show subdivisions for matrices over GF(2)

Reported by: jhpalmieri Owned by: jhpalmieri
Priority: minor Milestone: sage-3.4.1
Component: linear algebra Keywords:
Cc: Merged in:
Authors: Reviewers:
Report Upstream: Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

Since matrix_mod2_dense defines its own str function, it overrides the function for general matrices, so for dense matrices over GF(2), subdivisions are not printed. (See this discussion.)

This patch deletes the str method for dense matrices over GF(2).

Attachments (2)

5715-subdivisions.patch (1.2 KB) - added by jhpalmieri 11 years ago.
5715-mat2-subdiv.patch (2.3 KB) - added by robertwb 11 years ago.
apply only this patch

Download all attachments as: .zip

Change History (12)

Changed 11 years ago by jhpalmieri

comment:1 Changed 11 years ago by jhpalmieri

  • Resolution set to duplicate
  • Status changed from new to closed
  • Summary changed from [with patch, needs review] show subdivisions for matrices over GF(2) to show subdivisions for matrices over GF(2)

This is a duplicate of #5714.

comment:2 Changed 11 years ago by was

  • Summary changed from show subdivisions for matrices over GF(2) to [with patch; needs work] show subdivisions for matrices over GF(2)

Needs work. There is a *very good* reason that there is a str method for GF(2) -- it's for speed! Check this out:

BEFORE YOUR PATCH:
sage: a = random_matrix(GF(2),1000)
sage: time b = a.str()
CPU times: user 0.41 s, sys: 0.01 s, total: 0.42 s
Wall time: 0.42 s
AFTER YOUR PATCH:
sage: a = random_matrix(GF(2),1000)
sage: time b = a.str()
CPU times: user 5.02 s, sys: 0.86 s, total: 5.88 s
Wall time: 5.89 s

Changed 11 years ago by robertwb

apply only this patch

comment:3 Changed 11 years ago by robertwb

  • Summary changed from [with patch; needs work] show subdivisions for matrices over GF(2) to [with patch; needs review] show subdivisions for matrices over GF(2)

I posted a new patch that handles subdivisions. I went ahead and sped up the str method while I was there too.

Before

sage: a = random_matrix(GF(2),1000)
sage: time b = a.str()
CPU times: user 0.48 s, sys: 0.01 s, total: 0.49 s
Wall time: 0.50 s

After

sage: a = random_matrix(GF(2),1000)
sage: time b = a.str()
CPU times: user 0.01 s, sys: 0.01 s, total: 0.02 s
Wall time: 0.02 s

comment:4 Changed 11 years ago by mabshoff

Hmm, shouldn't this patch be reopened or maybe go to another ticket? We need to do one thing since this patch is currently closed as a dupe.

Cheers,

Michael

comment:5 Changed 11 years ago by robertwb

  • Resolution duplicate deleted
  • Status changed from closed to reopened

comment:6 Changed 11 years ago by robertwb

I just reopened this ticket, as it seems to have the most activity.

comment:7 Changed 11 years ago by jhpalmieri

  • Summary changed from [with patch; needs review] show subdivisions for matrices over GF(2) to [with patch; positive review] show subdivisions for matrices over GF(2)

Looks good, passes all tests when applied to 3.4.1.rc2. Also fast, as robertwb pointed out above.

A question: how do you tell if a sparse matrix is too big to be converted to a dense one? If we have a sparse mod 2 matrix, should we consider printing it by converting to a dense one and using this routine? For a 1000x1000 random matrix, converting and using this one was about 5 times faster than just printing it. (If this is a good idea, then it belongs in a different ticket, but I don't know if this is a good idea...)

comment:8 Changed 11 years ago by mabshoff

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

Merged 5715-mat2-subdiv.patch in Sage 3.4.1.rc3.

Cheers,

Michael

comment:9 Changed 11 years ago by robertwb

How big is too big is an interesting question, but too big to convert > to big to print. Probably should be a separate ticket, but the optimizer in me sees an even easier way: create the zero matrix doing "[0 0 ... 0]" * nrows, then set the 1 entries. This wouldn't of course handle subdivisons though, but could be easily adapted to do so the same way as this code.

comment:10 Changed 11 years ago by mabshoff

  • Milestone changed from sage-3.4.2 to sage-3.4.1
Note: See TracTickets for help on using tickets.