[ https://issues.apache.org/jira/browse/MAHOUT-790?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13088451#comment-13088451 ]
Ted Dunning commented on MAHOUT-790: ------------------------------------ I have also found a number of instances where from.addTo(to) is used instead of to.assign(from, Functions.PLUS). I am changing these usages to the latter form and removing addTo as redundant? Any comment on that change? > 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