Github user takuti commented on a diff in the pull request:
https://github.com/apache/incubator-hivemall/pull/115#discussion_r139072140
--- Diff:
core/src/main/java/hivemall/evaluation/BinaryResponsesMeasures.java ---
@@ -79,8 +91,15 @@ public static double IDCG(final int n) {
* @return Precision
*/
public static double Precision(@Nonnull final List<?> rankedList,
- @Nonnull final List<?> groundTruth, @Nonnull final int
recommendSize) {
- return (double) countTruePositive(rankedList, groundTruth,
recommendSize) / recommendSize;
+ @Nonnull final List<?> groundTruth, @Nonnegative final int
recommendSize) {
+ if (rankedList.isEmpty()) {
+ if (groundTruth.isEmpty()) {
+ return 1.d;
+ }
+ return 0.d;
+ }
+ final int k = Math.min(rankedList.size(), recommendSize);
--- End diff --
Computing `Math.min(rankedList.size(), recommendSize)` twice at here and
inside of `TruePositives` looks redundant and a little bit confusing. For the
sake of simplicity, how about directly writing what `TruePositives` does both
in `Precision` and `Recall`, and remove `TruePositives`? Even though the same
for-loop appears twice, I personally feel it's much more easier to understand.
---
