commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ismael Juma (JIRA)" <>
Subject [jira] Commented: (MATH-230) Implement Sparse Matrix Support
Date Sat, 06 Dec 2008 00:35:44 GMT


Ismael Juma commented on MATH-230:

I looked into this more carefully and I have a few comments (note that I don't speak for commons-math,
just an interested onlooker who might use it in the near future):

- There have been a few commits after the patch was generated that make it not apply anymore.

- I am not sure if extending RealMatrixImpl is the right approach. Stuff like the following
is a red flag.

public double[][] getData() {
        throw new UnsupportedOperationException(
            "Not supported because sparse matrices are usually chosen when " +
            "the matrix to be modeled is too large to fit into memory, trying " +
            "to build the data array from this may result in an OutOfMemoryException.");

Also, it seems to override most (if not all) of the methods. Is it perhaps better to introduce
an AbstractRealMatrix containing the shared methods and have both RealMatrixImpl and SparseRealMatrix
extend it? I notice that this won't fix the getData() issue though, would it make sense to
move that method into the dense matrix? (only possible if 2.0 can break compatibility).

- The following code in SparseRealMatrixImpl.setEntry does not need the containsKey call,
remove can be called directly (avoiding an unnecessary look-up):
            // if an entry exists, remove it
            if (entries.containsKey(point)) {

There is another case of containsKey that is unnecessary because it's followed by a get after
it, but once the specialised map exists the code will be even simpler so we can leave that
for later.

> Implement Sparse Matrix Support
> -------------------------------
>                 Key: MATH-230
>                 URL:
>             Project: Commons Math
>          Issue Type: Improvement
>    Affects Versions: 2.0
>         Environment: N/A
>            Reporter: Sujit Pal
>            Assignee: Luc Maisonobe
>            Priority: Minor
>             Fix For: 2.0
>         Attachments: patch.txt,,,
> I needed a way to deal with large sparse matrices using commons-math RealMatrix, so I
implemented it. The SparseRealMatrixImpl is a subclass of RealMatrixImpl, and the backing
data structure is a Map<Point,Double>, where Point is a struct like inner-class which
exposes two int parameters row and column. I had to make some changes to the existing components
to keep the code for SparseRealMatrixImpl clean. Here are the details.
> 1)
>    - added a new method setEntry(int, int, double) to set data into a matrix
> 2)
>    - changed all internal calls to data[i][j] to getEntry(i,j).
>    - for some methods such as add(), subtract(), premultiply(), etc, there
>      was code that checked for ClassCastException and had two versions,
>      one for a generic RealMatrix and one for a RealMatrixImpl. This has
>      been changed to have only one that operates on a RealMatrix. The 
>      result is something like auto-type casting. So if:
>        RealMatrixImpl.add(RealMatrix) returns a RealMatrixImpl
>        SparseRealMatrixImpl.add(RealMatrix) returns a SparseRealMatrixImpl
> 3) SparseRealMatrixImpl added as a subclass of RealMatrixImpl.
> 4) LUDecompositionImpl changed to use a clone of the passed in RealMatrix
>    instead of its data[][] block, and now it uses clone.getEntry(row,col)
>    calls instead of data[row][col] calls.
> 5) LUDecompositionImpl returned RealMatrixImpl for getL(), getU(), getP()
>    and solve(). It now returns the same RealMatrix impl that is passed 
>    in through its constructor for these methods.
> 6) New test for SparseRealMatrixImpl, mimics the tests in RealMatrixImplTest,
> 7) New static method to create SparseRealMatrixImpl out of a double[][] in
>    MatrixUtils.createSparseRealMatrix().
>    but using SparseRealMatrixImpl.
> 8) Verified that all JUnit tests pass.

This message is automatically generated by JIRA.
You can reply to this email to add a comment to the issue online.

View raw message