commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Sujit Pal <>
Subject Re: [math] patch submitted for commons-math sparse matrix support
Date Mon, 01 Dec 2008 23:01:36 GMT
Hi Luc,

Thanks for the feedback. I did not realize that the check for the
ClassCastException was performance related. Let me know your thoughts
once you get a chance to look at the patch, and I can put this back in
and resubmit, ie if it is a RealMatrixImpl, then do what the methods
with RealMatrixImpl did, else use the getEntry() calls.

Also, I think the JVM can detect that the incoming arg is a
RealMatrixImpl and go to the correct method without the code having to
catch a ClassCastException. I have used this pattern in some other code
I've written, where there are multiple overloaded methods, each taking a
particular impl.


On Mon, 2008-12-01 at 23:52 +0100, Luc Maisonobe wrote:
> Sujit Pal a écrit :
> > Hello,
> > 
> > I am a new, but fairly happy user of commons-math 1.2 and I have begun
> > to use it in my code. I needed a sparse matrix implementation for some
> > data analysis work I was doing, and unfortunately the code with
> > RealMatrixImpl was coming back with an OOME. So I built a
> > SparseRealMatrixImpl as a subclass of RealMatrixImpl, which uses a
> > Map<Point,Double> as its backing store, storing only the non-zero
> > entries. I would like to contribute this implementation back to the
> > commons-math project.
> > 
> > I have opened a JIRA enhancement request MATH-230 with a patch against
> > the 2.0 branch. Patch and detailed description of the changes are
> > available here.
> >
> > 
> > I have also had to make some changes to RealMatrixImpl and
> > LUDecompositionImpl, mainly replacing the data[][] references with
> > RealMatrix.getEntry(i,j) calls to insulate the backing store from being
> > accessed directly by calling code, and thus making it possible for
> > subclasses to provide their own internal backing stores.
> > 
> > Would appreciate if you folks can take a look and let me know if it is
> > feasible to add this into the 2.0 release. That way I can continue using
> > commons-math future versions without having to patch it after every
> > upgrade.
> I'll have a look at it tomorrow evening.
> For now, one thing  that bother me is the removal of the dedicated code
> that explicitely avoided getEntry(). This was added a few months ago for
> performance reason, since the previous generic code was painfully slow.
> The trick with the ClassCastException allows really fast check since the
> virtual machine can completely optimize it out in some cases, it is an
> enhancement of what was discussed here:
> Our discussion at that
> time was that the more common case (RealMatrixImpl) should be as
> efficient as possible (and Ted wants now it to be even faster by
> changing the underlying storage which is a good thing). This trick is
> not beautiful perfectly generic code, it is a pragmatic way to be fast
> in a common case and removing it will most probably induce poorer
> performances.
> Luc
> > 
> > I noticed that there has been some previous discussion about sparse
> > matrix support here:
> >
> > so I sent a personal email to John Iacona asking him about about the
> > code he speaks of here. But he is working on other things, and doesn't
> > know when he will be able to check in his sparse matrix support changes.
> > His approach is a little  different from mine (he has a SparseMatrix
> > interface, which is implemented by a SparseMatrixImpl). I considered
> > that approach initially, but came to the conclusion that it would mean
> > quite a bit of code duplication, which I wanted to avoid as far as
> > possible.
> > 
> > Thanks
> > -sujit
> > 
> > 
> > 
> > 
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail:
> > For additional commands, e-mail:
> > 
> > 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail:
> For additional commands, e-mail:

To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message