Mailing-List: contact dev-help@mahout.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@mahout.apache.org
Date: Mon, 22 Aug 2011 17:03:29 +0000 (UTC)
From: "Sean Owen (JIRA)" <jira@apache.org>
To: dev@mahout.apache.org
Message-ID: 
 <1789827759.1671.1314032609778.JavaMail.tomcat@hel.zones.apache.org>
In-Reply-To: 
 <380417812.56076.1313875707240.JavaMail.tomcat@hel.zones.apache.org>
Subject: [jira] [Commented] (MAHOUT-790) Redundancy in Matrix API, view or
 get?
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


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

Sean Owen commented on MAHOUT-790:
----------------------------------

I may misunderstand: Do you expect m and m.viewPart() to be the same class in an ideal world or not? I don't expect that, myself, and indeed that's not the case. So all the less would I expect that m.like() and m.viewPart().like() ought to be the same class. I thought you were suggesting they ought to be.

I agree with your last point here so I think we must be agreeing.

> Redundancy in Matrix API, view or get?
> --------------------------------------
>
>                 Key: MAHOUT-790
>                 URL: https://issues.apache.org/jira/browse/MAHOUT-790
>             Project: Mahout
>          Issue Type: Improvement
>    Affects Versions: 0.5
>            Reporter: Ted Dunning
>             Fix For: 0.6
>
>         Attachments: MAHOUT-790.patch
>
>
> We have a bunch of redundant methods in our matrix interface.  These include things that return views of parts of the matrix:
> {code}
>   Matrix viewPart(int[] offset, int[] size);
>   Matrix viewPart(int rowOffset, int rowsRequested, int columnOffset, int columnsRequested);
>   Vector viewRow(int row);
>   Vector viewColumn(int column);
> {code}
> and things that do the same but call refer to getting stuff
> {code}
>   Vector getColumn(int column);
>   Vector getRow(int row);
>   double getQuick(int row, int column);
>   int[] getNumNondefaultElements();
>   Map<String, Integer> getColumnLabelBindings();
>   Map<String, Integer> getRowLabelBindings();
>   double get(String rowLabel, String columnLabel);
> {code}
> To my mind, get implies a get-by-value whereas view implies get-by-reference.  As such, I would suggest that getColumn and getRow should disappear.  On the other hand, getQuick and get are both correctly named.  
> This raises the question of what getNumNondefaultElements really does.  I certainly can't tell just from the signature.  Is it too confusing to keep?
> Additionally, what do people think that getColumnLabelBindings and getRowLabelBindings return?  A mutable map?  Or an immutable one?
> Under the covers, viewRow and viewColumn (and the upcoming viewDiagonal) have default implementations that use MatrixVectorView, but AbstractMatrix doesn't have an implementation for getRow and getColumn. 
> In sum, I suggest that:
>   - getRow and getColumn go away
>   - the fancy fast implementations fo getRow and getColumn that exist be migrated to be over-rides of viewRow and viewColumn
>   - there be a constructor for AbstractMatrix that sets the internal size things correctly.
>   - that the internal cardinality array in AbstractMatrix goes away to be replaced by two integers.
>   - viewDiagonal() and viewDiagonal(length) and viewDiagonal(row, column) and viewDiagonal(int row, column, length) be added.
> I will produce a patch shortly.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira