You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Phil Steitz <ph...@steitz.com> on 2004/10/11 20:14:19 UTC
[math] RealMatrixImpl changes was: RE: cvs commit: jakarta-commons/math/src/test/org/apache/commons/math/linear RealMatrixImplTest.java
Mark,
I now see (more fully) what you mean about the extra copy operations and agree that there is a little more to do than what I responded below. Using getElement and getDataRef for the current instance should suffice, however, in most cases. I need to understand better what you are proposing.
Phil
-----Original Message-----
From: Phil Steitz
Sent: Mon 10/11/2004 10:16 AM
To: Jakarta Commons Developers List
Cc:
Subject: Re: cvs commit: jakarta-commons/math/src/test/org/apache/commons/math/linear RealMatrixImplTest.java
Mark R. Diggory wrote:
> Hey Phil,
>
> Thanks for working on the RealMatrixImpl, it really gets things rolling.
> I was working on some things offline to support it further. One point of
> concern I have is that I thought we were going to make RealMatrixImpl
> "immutable" and allow the submatrix accessors return implementations
> that reference the internal matrix structure.
Yes, I was planning to remove the setters next.
>
> So, currently I would recommend our using an implementation of
> RealMatrix that maintains a reference to the original data[][] array and
> the submatix dimensions it is working within on that matrix. I have been
> working on such an implementation upto this point. Your adding the
> methods to the interface is good and I'll start working on
> implementations that use this strategy.
What do you mean here? I agree with J that getDataRef should be supported
only in the RealMatrixImpl (where I would leave it in)
>
> I also notice in the implementations there a great degree of "copying"
> by using the "getData() method" going on during the processing of
> methods such as add,subtract etc.
>
> getData()
> http://jakarta.apache.org/commons/math/xref/org/apache/commons/math/linear/RealMatrixImpl.html#256
>
getData() is supposed to return a copy of the underlying data. That is
its contract.
>
> which calls copyOut(), copying the internal array just to access its
> contents
> http://jakarta.apache.org/commons/math/xref/org/apache/commons/math/linear/RealMatrixImpl.html#821
>
>
> add()
> http://jakarta.apache.org/commons/math/xref/org/apache/commons/math/linear/RealMatrixImpl.html#141
>
Yes, that should be fixed. getDataRef should be used in place of getData
to get the data for the instance.
>
> This seems unnecessary, I think using methods to access the array
> contents of the operand matrix would alleviate this unneeded copying.
> For instance:
>
>> for (int row = 0; row < rowCount; row++) {
>> for (int col = 0; col < columnCount; col++) {
>> outData[row][col] = data[row][col] + m.getEntry(row,col);
>> }
>> }
>
That would work in the current impl, actually even better than using
getDataRef.
>
> The modifications I am working on include these fixes and produce a
> separate abstract class "AbstractRealMatrix" which supports "storage
> independent" versions of these methods. I'm working to use this Abstract
> implementation to form the basis for RealMatrixImpl and any
> "internalized" versions used to support the column, Row and Submatrices.
Do we really need this? Do we need it for 1.0? Please provide more details.
>
Phil
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [math] RealMatrixImpl changes was: RE: cvs commit: jakarta-commons/math/src/test/org/apache/commons/math/linear
RealMatrixImplTest.java
Posted by "Mark R. Diggory" <md...@latte.harvard.edu>.
Phil,
Heres a patch that shows the changes which would solve this copying
issue in the current RealMatrixImpl.
Index: RealMatrixImpl.java
===================================================================
RCS file:
/home/cvs/jakarta-commons/math/src/java/org/apache/commons/math/linear/RealMatrixImpl.java,v
retrieving revision 1.32
diff -u -r1.32 RealMatrixImpl.java
--- RealMatrixImpl.java 10 Oct 2004 18:00:33 -0000 1.32
+++ RealMatrixImpl.java 11 Oct 2004 19:09:03 -0000
@@ -48,7 +48,7 @@
*
* @version $Revision: 1.32 $ $Date: 2004/10/10 18:00:33 $
*/
-public class RealMatrixImpl implements RealMatrix, Serializable {
+public class RealMatrixImpl implements RealMatrix,
InternalMatrixSupport, Serializable {
/** Serializable version identifier */
static final long serialVersionUID = 4237564493130426188L;
@@ -160,7 +160,7 @@
int rowCount = this.getRowDimension();
int columnCount = this.getColumnDimension();
double[][] outData = new double[rowCount][columnCount];
- double[][] mData = m.getData();
+ double[][] mData = ((InternalMatrixSupport)m).getDataRef();
for (int row = 0; row < rowCount; row++) {
for (int col = 0; col < columnCount; col++) {
outData[row][col] = data[row][col] + mData[row][col];
@@ -184,7 +184,7 @@
int rowCount = this.getRowDimension();
int columnCount = this.getColumnDimension();
double[][] outData = new double[rowCount][columnCount];
- double[][] mData = m.getData();
+ double[][] mData = ((InternalMatrixSupport)m).getDataRef();
for (int row = 0; row < rowCount; row++) {
for (int col = 0; col < columnCount; col++) {
outData[row][col] = data[row][col] - mData[row][col];
@@ -242,7 +242,7 @@
int nRows = this.getRowDimension();
int nCols = m.getColumnDimension();
int nSum = this.getColumnDimension();
- double[][] mData = m.getData();
+ double[][] mData = ((InternalMatrixSupport)m).getDataRef();
double[][] outData = new double[nRows][nCols];
double sum = 0;
for (int row = 0; row < nRows; row++) {
@@ -716,7 +716,7 @@
int nRowB = b.getRowDimension();
// Apply permutations to b
- double[][] bv = b.getData();
+ double[][] bv = ((InternalMatrixSupport)b).getDataRef();
double[][] bp = new double[nRowB][nColB];
for (int row = 0; row < nRowB; row++) {
for (int col = 0; col < nColB; col++) {
Index: InternalMatrixSupport.java
===================================================================
RCS file: InternalMatrixSupport.java
diff -N InternalMatrixSupport.java
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ InternalMatrixSupport.java 1 Jan 1970 00:00:00 -0000
@@ -0,0 +1,26 @@
+/*
+ * Created on Oct 11, 2004
+ *
+ * TODO To change the template for this generated file go to
+ * Window - Preferences - Java - Code Style - Code Templates
+ */
+package org.apache.commons.math.linear;
+
+/**
+ * @author Administrator
+ *
+ * TODO To change the template for this generated type comment go to
+ * Window - Preferences - Java - Code Style - Code Templates
+ */
+public interface InternalMatrixSupport {
+
+ /**
+ * Returns a reference to the underlying data array.
+ * <p>
+ * Does not make a fresh copy of the underlying data.
+ *
+ * @return 2-dimensional array of entries
+ */
+ public double[][] getDataRef();
+
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [math] RealMatrixImpl changes was: RE: cvs commit: jakarta-commons/math/src/test/org/apache/commons/math/linear
RealMatrixImplTest.java
Posted by "Mark R. Diggory" <md...@latte.harvard.edu>.
Phil Steitz wrote:
> Mark,
>
> I now see (more fully) what you mean about the extra copy operations and agree that there is a little more to do than what I responded below. Using getElement and getDataRef for the current instance should suffice, however, in most cases. I need to understand better what you are proposing.
>
> Phil
>
> -----Original Message-----
> From: Phil Steitz
> Sent: Mon 10/11/2004 10:16 AM
> To: Jakarta Commons Developers List
> Cc:
> Subject: Re: cvs commit: jakarta-commons/math/src/test/org/apache/commons/math/linear RealMatrixImplTest.java
>
>
>
> Mark R. Diggory wrote:
> > Hey Phil,
> >
> > Thanks for working on the RealMatrixImpl, it really gets things rolling.
> > I was working on some things offline to support it further. One point of
> > concern I have is that I thought we were going to make RealMatrixImpl
> > "immutable" and allow the submatrix accessors return implementations
> > that reference the internal matrix structure.
>
> Yes, I was planning to remove the setters next.
Ok
> >
> > So, currently I would recommend our using an implementation of
> > RealMatrix that maintains a reference to the original data[][] array and
> > the submatix dimensions it is working within on that matrix. I have been
> > working on such an implementation upto this point. Your adding the
> > methods to the interface is good and I'll start working on
> > implementations that use this strategy.
>
> What do you mean here? I agree with J that getDataRef should be supported
> only in the RealMatrixImpl (where I would leave it in)
Yes, what I've been attempting is to isolate the data[][] store from the
methods so that a method like getDataRef() can actually return the
data[][] of the original parent matrix.
Then methods such as add(RealMatrix m) actually use information about
the "bounds"
> public RealMatrix add(RealMatrix m) throws IllegalArgumentException {
>
> int rowCount = this.getRowDimension();
> int columnCount = this.getColumnDimension();
>
> if (columnCount != m.getColumnDimension() ||
> rowCount != m.getRowDimension()) {
> throw new IllegalArgumentException("matrix dimension mismatch");
> }
>
> double[][] data = this.getDataRef();
>
> double[][] outData = new double[rowCount][columnCount];
>
> for (int row = 0; row < rowCount; row++) {
> for (int col = 0; col < columnCount; col++) {
> outData[row][col] = data[row + ROWMIN][col + COLMIN] + m.getEntry(row,col);
> }
> }
> return new RealMatrixImpl(outData);
>
> }
where the implementation maintains what the ROWMIN, ROWMAX, COLMIN,
COLMAX. and transposes it when something like getEntry(x,y) is called by
doing something like.
> public double getEntry(int row, int column)
> throws MatrixIndexException {
> return getDataRef()[row + ROWMIN][column + COLMIN];
> }
>
> >
> > I also notice in the implementations there a great degree of "copying"
> > by using the "getData() method" going on during the processing of
> > methods such as add,subtract etc.
> >
> > getData()
> > http://jakarta.apache.org/commons/math/xref/org/apache/commons/math/linear/RealMatrixImpl.html#256
> >
> getData() is supposed to return a copy of the underlying data. That is
> its contract.
Yes, but it is a copy, not the original, I think we agree that to
enforce immutability it should remain so.
> >
> > which calls copyOut(), copying the internal array just to access its
> > contents
> > http://jakarta.apache.org/commons/math/xref/org/apache/commons/math/linear/RealMatrixImpl.html#821
> >
> >
> > add()
> > http://jakarta.apache.org/commons/math/xref/org/apache/commons/math/linear/RealMatrixImpl.html#141
> >
> Yes, that should be fixed. getDataRef should be used in place of getData
> to get the data for the instance.
Yes, but we are restricted to casting to some implementation of
getDataRef because we do not want it exposed in the Interface so we need
to make sure the method is available in the implementation. We might do
this by establishing and internal service interface that all
Implementations must also meet.
public interface InternalMatrixSupport {
public double[][] getDataRef();
}
and require all implementations to support it
public class RealMatrixImpl implements RealMatrix, InternalMatrixSupport{
}
> >
>
> > This seems unnecessary, I think using methods to access the array
> > contents of the operand matrix would alleviate this unneeded copying.
> > For instance:
> >
> >> for (int row = 0; row < rowCount; row++) {
> >> for (int col = 0; col < columnCount; col++) {
> >> outData[row][col] = data[row][col] + m.getEntry(row,col);
> >> }
> >> }
> >
> That would work in the current impl, actually even better than using
> getDataRef.
Yes, we can take this step now prior to release.
> >
> > The modifications I am working on include these fixes and produce a
> > separate abstract class "AbstractRealMatrix" which supports "storage
> > independent" versions of these methods. I'm working to use this Abstract
> > implementation to form the basis for RealMatrixImpl and any
> > "internalized" versions used to support the column, Row and Submatrices.
>
> Do we really need this? Do we need it for 1.0? Please provide more details.
I'd like to see it if I can implement the Abstract version in time. But
if scheduling doesn't permit, we can move forward without it. We should
just make sure we at least get the optimizations we discussed for
RealMatrixImpl in place.
--
Mark Diggory
Software Developer
Harvard MIT Data Center
http://www.hmdc.harvard.edu
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org