You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mahout.apache.org by "Ted Dunning (JIRA)" <ji...@apache.org> on 2009/12/02 01:16:20 UTC

[jira] Commented: (MAHOUT-211) DenseMatrix.getRow() and getColumn() return deep copies of the data

    [ https://issues.apache.org/jira/browse/MAHOUT-211?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12784557#action_12784557 ] 

Ted Dunning commented on MAHOUT-211:
------------------------------------


The Colt approach is definitely to go full-bore on the shallow reference semantics.  In fact, things like sub-matrix and transpose primarily return views which allows all kinds of interesting compositional coding practices.  As a trivial example, there is no primitive in Colt for setting the diagonal of a matrix to zero.  There is a primitive for getting a view of the diagonal as a vector and there is a primitive for setting all elements of a vector to zero.  These can easily be composed to get the desired result.

In some areas, this style is extremely effective, but it can also be very confusing (there is Matrix.plus, for instance).  I have found that some users need a more traditional API for getting started, but that is easily done.

> DenseMatrix.getRow() and getColumn() return deep copies of the data
> -------------------------------------------------------------------
>
>                 Key: MAHOUT-211
>                 URL: https://issues.apache.org/jira/browse/MAHOUT-211
>             Project: Mahout
>          Issue Type: Bug
>          Components: Matrix
>    Affects Versions: 0.1
>         Environment: all
>            Reporter: Jake Mannix
>            Priority: Minor
>             Fix For: 0.3
>
>
> As mentioned in the summary, DenseMatrix.getRow() returns a new DenseVector(values[row]) instead of just a shallow reference.  This is a) wasteful, and b) inconsistent with what SparseRowMatrix / SparseColumnMatrix do, which means that users who think they can mutate rows by calling getRow() are wrong for DenseMatrix, or conversely think they can modify the row at will are wrong for SparseRowMatrix.  
> Either way, we need to be consistent.  I would suggest the contract be that you get a shallow view reference (meaning, it may be an actual reference to a real Vector, or it may be a virtual Vector, dynamically constructed, which is still backed by entries in the matrix), so that calling Matrix.getRow(row).set(col, value) has the same affect as Matrix.set(row, col, value).  If we don't do it this way, we should at least provide an api to get a shallow reference to rows.
> Thoughts?

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