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

Ted Dunning commented on MAHOUT-790:
------------------------------------

{quote}
Diagonals: are they really needed now? 
{quote}
It really helps some algorithms to be able to pull the primary diagonal of a 
matrix out as a vector.  So yes, that is needed.

Regarding a DiagonalMatrix, I do have a need for that as well and will include 
that in the SSVD patch.  Since that is a pure addition, I don't think it needs 
the same level of discussion.

{quote}
Should there be triangular or symmetric? They have enough of their own behavior 
to be a separate subclass rather than some magic thing held by the main class. 
{quote}
Possibly.  So far, this isn't a big deal although I do have a triangular solver 
in my CholeskyDecomposition.  If we see some more uses, it might be worth 
pulling out as a separate class.

I don't see that a SymmetricMatrix is an important thing yet.  Yes, there are 
important mathematical properties there, but these aren't necessarily something 
worth reflecting in the type structure.  Good use cases would change my mind.

{quote}
Example: DiagonalMatrix.invert() is a valid method, because it either blows up 
if there is a 0 value, or returns 1/values.
{quote}
I prefer solve methods rather than invert methods.  It is already much too hard 
to cure people of trying to invert matrices.  Introducing an invert method 
anywhere would just make that harder.

> Redundancy in Matrix API, view or get?
> --------------------------------------
>
>                 Key: MAHOUT-790
>                 URL: https://issues.apache.org/jira/browse/MAHOUT-790
>             Project: Mahout
>          Issue Type: Improvement
>            Reporter: Ted Dunning
>
> 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

        

Reply via email to