[
https://issues.apache.org/jira/browse/MAHOUT-790?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13088760#comment-13088760
]
Ted Dunning commented on MAHOUT-790:
------------------------------------
Sean,
We definitely need both operations. The first can be expressed, however, as
a.like().assign(a) so it isn't quite as necessary as it might seem.
The problem with clone itself is that there are serious restrictions on how you
have to do it based on Java requirements. That makes it a royal pain some days
of the week. This may be easier after this JIRA gets resolved since the only
information at AbstractMatrix level is the number of rows and columns and they
are trivial to deal with.
We definitely also have some bugs in our test suite in that it is assumed that
like() has to return the same type of object. That isn't really true. For
instance, m.viewPart(0,3,2,5).like() should return the same thing that m.like()
returns. But viewPart probably returns some kind of view object so these
aren't the same.
I can deal with those issues another day.
If we can get eyes on this monster patch, I would like to commit it shortly.
The only big issues I know about are:
a) getRow and getColumn is a more common name than viewRow and viewColumn.
Does everybody promise not to be confused by the loss of getRow?
b) what should the iterator of a matrix do? Right now SparseColumnMatrix
iterates over columns and everything else by rows. Unless there is some very
clear way to tell what the iteration is doing, I would like to go on record as
grumpy about that.
> 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
