#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)
Change History (12)
Changed 11 years ago by
comment:1 Changed 11 years ago by
- 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)
comment:2 Changed 11 years ago by
- 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
comment:3 Changed 11 years ago by
- 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
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
- Resolution duplicate deleted
- Status changed from closed to reopened
comment:6 Changed 11 years ago by
I just reopened this ticket, as it seems to have the most activity.
comment:7 Changed 11 years ago by
- 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
- 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
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
- Milestone changed from sage-3.4.2 to sage-3.4.1
This is a duplicate of #5714.