[ 
https://issues.apache.org/jira/browse/MATH-230?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12653887#action_12653887
 ] 

Sujit Pal commented on MATH-230:
--------------------------------

>> Or we could just add the appropriate specialised Map to commons-math. Are 
>> there objections to that?
I cannot answer this one, since I am not a developer on the project - my 
observation is that there are no dependencies for this project save the JDK, 
which leads me to believe that there may have been a concious decision to keep 
it that way, but I could be wrong. As for adding it into commons-math, my 
personal opinion is that it belongs in a data structure package such as 
commons-collections or such, but thats just my view.

>> I think it's important to provide an implementation that has the right 
>> performance characteristics 
>> by default.
I completely agree. And the open/chained hashmap would definitely be more 
appropriate in a memory constrained environment where a SparseMatrix would be 
considered, than my Map<Point,Double>.

And if it is implemented as a private inner class, then it is visible only from 
within the implementation, so I think there should be no issues. And it would 
be completely justifiable, since we are basically creating a fast data 
structure to serve the impl.

However, it is already assigned to be reviewed, so I am guessing that when Luc 
gets to this bug, he will post his review. FWIW, I think that if we do end up 
including the SparseMatrixImpl, then definitely your approach with the 
open/chained hashmap would be preferable to the one thats in there. If you have 
th bandwidth to build it, then I think it may be quite helpful.







> 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: 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.

Reply via email to