Opened 11 years ago

Closed 11 years ago

#9856 closed enhancement (fixed)

improve `augment` method for sparse matrices

Reported by: ylchapuy Owned by: jason, was
Priority: major Milestone: sage-4.6
Component: linear algebra Keywords:
Cc: zimmerma Merged in: sage-4.6.alpha2
Authors: Yann Laigle-Chapuy Reviewers: Paul Zimmermann
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

see #7199 for a patch improving stack.

Attachments (2)

trac9856-improve_augment_for_sparse_matrices.patch (2.7 KB) - added by ylchapuy 11 years ago.
trac9856-typo_fix.patch (799 bytes) - added by ylchapuy 11 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 11 years ago by ylchapuy

  • Authors set to Yann Laigle-Chapuy
  • Cc zimmerma added
  • Status changed from new to needs_review

Patch based on sage 4.5.3 + #7199 (but I don't know if it depends on it). Paul, as you did such a good job reviewing the other ticket, I cc'd you.

comment:2 Changed 11 years ago by zimmerma

  • Status changed from needs_review to needs_info

Paul, as you did such a good job reviewing the other ticket, I cc'd you.

Yann, please could you provide a description saying in what sense you did "improve" augment, maybe with an example? Is that an improvement in functionality or speed?

Paul

comment:3 Changed 11 years ago by ylchapuy

  • Status changed from needs_info to needs_review

It's a speed improvement. Here's the example:

sage: m = identity_matrix(QQ, 1000, sparse=True)
sage: timeit('m.augment(m)')

BEFORE

5 loops, best of 3: 368 ms per loop

AFTER

625 loops, best of 3: 1.2 ms per loop

And we are not loosing anything for small cases:

sage: m = identity_matrix(QQ, 5, sparse=True)  
sage: timeit('m.augment(m)')

BEFORE

625 loops, best of 3: 198 µs per loop

AFTER

625 loops, best of 3: 197 µs per loop

comment:4 Changed 11 years ago by zimmerma

  • Reviewers set to Paul Zimmermann
  • Status changed from needs_review to needs_work
  • Work issues set to typo in error message

A small typo in the code: "number of columns must be the same" should be "number of rows must be the same". I confirm the great speed improvement. Once the typo is fixed, I will check the doctests still pass.

Paul

Changed 11 years ago by ylchapuy

comment:5 Changed 11 years ago by ylchapuy

  • Status changed from needs_work to needs_review

Nice spot, typo fixed. Apply both patches.

Yann

comment:6 Changed 11 years ago by zimmerma

  • Status changed from needs_review to positive_review
  • Work issues typo in error message deleted

good work once again, Yann!

Paul

comment:7 Changed 11 years ago by mpatel

  • Merged in set to sage-4.6.alpha2
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.