[HIVEMALL-151] Support Matrix conversion from DoK to CSR/CSC matrix #121
Conversation
…upported Java version from 1.7 to 1.8
|
Review comments are welcome @takuti Still work in progress
|
|
|
@maropu could you review my PFOR implementation c0f465a if possible? |
myui
changed the title
|
Decided to avoid using DoK => CSR conversion for dataMatrix because rows are also sparse and not suited for CSR. |
| coo2csr(rows, cols, data, rowPointers, colIndicies, values, numRows, numCols, nnz); | ||
|
|
||
| if (sortColumns) { | ||
| sortIndicies(rowPointers, colIndicies, values); |
There was a problem hiding this comment.
I found lots of typos indicies in many different classes...it's indices
There was a problem hiding this comment.
found other typos. add_field_indicies should be add_field_indcies.
| + " - " + rowStart + " = " + numCols + " at i=" + i); | ||
| } | ||
|
|
||
| final IntDoublePair[] pairs = new IntDoublePair[numCols]; |
There was a problem hiding this comment.
Why don't you use existing hivemall.utils.struct.Pair instead of newly introduced IntDoublePair (and IntFloatPair) instance?
We frequently like to use pair/tuple-ish data structure, so I feel using the same interface struct.Pair as much as we can is a better idea.
final List<Pair<Integer, Double>> pairs = new ArrayList<Pair<Integer, Double>>();
for (int jj = rowStart; jj < rowEnd; jj++) {
pairs.add(Pair.of(colIndicies[jj], values[jj]));
}
Collections.sort(pairs, new Comparator<Pair<Integer, Double>>() {
@Override
public int compare(Pair<Integer, Double> x, Pair<Integer, Double> y) {
return Integer.compare(x.getKey(), y.getKey());
}
});
for (int jj = rowStart, n = 0; jj < rowEnd; jj++, n++) {
Pair<Integer, Double> tmp = pairs.get(n);
colIndicies[jj] = tmp.getKey();
values[jj] = tmp.getValue();
}There was a problem hiding this comment.
^ code is inefficient and better to use primitive objects.
Avoid Autoboxing and primitive <=> object conversions.
| } | ||
| } | ||
|
|
||
| private static void sortIndicies(@Nonnull final int[] rowPointers, |
There was a problem hiding this comment.
Since this method is used both for CSR and CSC, more generic argument names make the code more understandable e.g., majorAxisPointers and minorAxisIndices
What changes were proposed in this pull request?
What type of PR is it?
Improvement, Refactoring
What is the Jira issue?
https://issues.apache.org/jira/browse/HIVEMALL-151
How was this patch tested?
unit tests, manual tests
Checklist
(Please remove this section if not needed; check
xfor YES, blank for NO)mvn formatter:format, for your commit?