Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sparse linear algebra: nonzero_positions is slow #4648

Closed
boothby opened this issue Nov 29, 2008 · 4 comments
Closed

sparse linear algebra: nonzero_positions is slow #4648

boothby opened this issue Nov 29, 2008 · 4 comments

Comments

@boothby
Copy link

boothby commented Nov 29, 2008

Currently, generic sparse matrices inherit their nonzero_positions method from matrix0.py. This should be trivial to fix.

def nonzero_positions(self):
    return self._entries.keys()

Component: linear algebra

Keywords: sparse

Issue created by migration from https://trac.sagemath.org/ticket/4648

@boothby boothby self-assigned this Nov 29, 2008
@sagetrac-mabshoff sagetrac-mabshoff mannequin added this to the sage-3.2.2 milestone Nov 29, 2008
@rlmill
Copy link
Mannequin

rlmill mannequin commented Jan 23, 2009

Attachment: trac-4648.patch.gz

@rlmill
Copy link
Mannequin

rlmill mannequin commented Jan 23, 2009

comment:2

Not quite so trivial as two lines, but...

@rlmill rlmill mannequin added the s: needs review label Jan 23, 2009
@craigcitro
Copy link
Member

comment:3

Code looks good. Merge!

However, I think that the fact that all three versions of this code are nearly identical means that a new enhancement ticket should be opened to clean up the classes here. In particular, I think the following might be a reasonable plan:

  • in matrix_integer_sparse, rename _matrix to _rows for consistency (and clarity -- you're getting a list of rows, not a pointer to the whole matrix)
  • clean up the associated vector classes (in fact, vector_modn_sparse isn't even a class right now!), and have them all inherit from an abstract class with the same methods they all share (which could all raise NotImplementedErrors, for all that matters)
  • make each of the sparse matrix classes have a member _rows of type vector_sparse_generic or whatever, and then move the _nonzero_positions_by_row down to the generic class.

It would get rid of this code duplication, clean things up, etc.

@sagetrac-mabshoff
Copy link
Mannequin

sagetrac-mabshoff mannequin commented Jan 24, 2009

comment:4

Merged in Sage 3.3.alpha2.

Cheers,

Michael

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants