[ https://issues.apache.org/jira/browse/MATH-230?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12655868#action_12655868 ]
Sujit Pal commented on MATH-230: -------------------------------- Apologies for not getting back earlier on this, I had a chance to look at the diff this morning. I will work on making the SparseMatrix tests pass this evening and post the patch tomorrow morning. However, looking at the patch, I notice that this method is used to find the key to the internal map for storing the entry: + private int computeKey(int row, int column) { + return row * columnDimension + column; + } This effectively flattens out the matrix so a matrix like this: 1 2 3 4 5 6 7 8 9 10 11 12 is flattened out to: 1 2 3 4 5 6 7 8 9 10 11 12 Now if you wanted to look for entry (1,2) you look for entry (1*4 + 2) = 6. So we will always get a unique key for a given matrix position, given that by specifying the row and column dimension we are always specifying a fixed size rectangular matrix. This is quite beautiful and clever (note to Ismael: Thanks for doing this, and I wish I had thought of this :-)). But the question is: do we still need an open-addressed map structure? It seems to me that we can now just represent the sparse matrix internally with a Map<Integer,Double>? That way we don't even have to think about whether we want to put it as an inner class or in utils. Thoughts? > Implement Sparse Matrix Support > ------------------------------- > > Key: MATH-230 > URL: https://issues.apache.org/jira/browse/MATH-230 > 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: math-230.diff, patch.txt, > RealMatrixImplPerformanceTest.java, SparseRealMatrixImpl.java, > SparseRealMatrixImplTest.java > > > 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) RealMatrix.java: > - added a new method setEntry(int, int, double) to set data into a matrix > 2) RealMatrixImpl.java: > - 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.