[Core] Fix OSPDT review issues (oap-project#339)

* Fix OSPDT issues (oap-project#334) * upgrade to 1.6.0 Signed-off-by: minmingzhu <[email protected]> * fixed OSPDT issues Signed-off-by: minmingzhu <[email protected]> * update Signed-off-by: minmingzhu <[email protected]> * update service.cpp Signed-off-by: minmingzhu <[email protected]> * Empty-Commit * test pyspark CI Signed-off-by: minmingzhu <[email protected]> * rollback ci-standalone-test-cpu.sh Signed-off-by: minmingzhu <[email protected]> * split CI script to python and scala Signed-off-by: minmingzhu <[email protected]> * Update ci-tests.yml --------- Signed-off-by: minmingzhu <[email protected]> * [ML-335] Fix issues (oap-project#337) * split CI script to python and scala Signed-off-by: minmingzhu <[email protected]> * fixed issues Signed-off-by: minmingzhu <[email protected]> * update ci-tests.yml Signed-off-by: minmingzhu <[email protected]> * update service.cpp Signed-off-by: minmingzhu <[email protected]> * update service.cpp code style Signed-off-by: minmingzhu <[email protected]> --------- Signed-off-by: minmingzhu <[email protected]> * split CI script to python and scala Signed-off-by: minmingzhu <[email protected]> * update ci-tests.yml Signed-off-by: minmingzhu <[email protected]> * update service.cpp Signed-off-by: minmingzhu <[email protected]> * update service.cpp code style Signed-off-by: minmingzhu <[email protected]> * fix issues Signed-off-by: minmingzhu <[email protected]> * fixed OSPDT review issues Signed-off-by: minmingzhu <[email protected]> * update RowAccessorImpl.cpp Signed-off-by: minmingzhu <[email protected]> * update suite test Signed-off-by: minmingzhu <[email protected]> * update Signed-off-by: minmingzhu <[email protected]> --------- Signed-off-by: minmingzhu <[email protected]>
minmingzhu · Aug 21, 2023 · 20f2fd6 · 20f2fd6
1 parent ef71e29
commit 20f2fd6
Show file tree

Hide file tree

Showing 32 changed files with 345 additions and 84 deletions.
diff --git a/.github/workflows/ci-tests.yml b/.github/workflows/ci-tests.yml
@@ -73,8 +73,8 @@ jobs:
       - name: Cluster Test
         run: |
           ${{github.workspace}}/dev/ci/ci-yarn-test-cpu.sh
-  standalone-test:
-    name: Standalone CPU_GPU_PROFILE Test for Examples (CPU)
+  standalone-scala-test:
+    name: Standalone CPU_GPU_PROFILE Test for scala Examples (CPU)
     runs-on: ubuntu-20.04
     steps:
       - uses: actions/checkout@v3
@@ -95,4 +95,28 @@ jobs:
             ${{ runner.os }}-
       - name: Cluster Test
         run: |
-          ${{github.workspace}}/dev/ci/ci-standalone-test-cpu.sh
+          ${{github.workspace}}/dev/ci/ci-standalone-scala-test-cpu.sh
+  standalone-python-test:
+    name: Standalone CPU_GPU_PROFILE Test for python Examples (CPU)
+    runs-on: ubuntu-20.04
+    steps:
+      - uses: actions/checkout@v3
+      - name: Set up JDK 1.8
+        uses: actions/setup-java@v1
+        with:
+          java-version: 1.8
+      - name: Restore cached dependencies
+        uses: actions/cache@v3
+        with:
+          path: |
+            #/var/cache/apt/archives/*.deb
+            ~/.m2/repository
+            /opt/intel/oneapi
+            ~/opt
+          key: ${{ runner.os }}_spark-3.2.0_hadoop-3.2.0_oneapi-2023.0.0
+          restore-keys: |
+            ${{ runner.os }}-
+      - name: Cluster Test
+        run: |
+          ${{github.workspace}}/dev/ci/ci-standalone-python-test-cpu.sh
+          
diff --git a/dev/ci/ci-standalone-python-test-cpu.sh b/dev/ci/ci-standalone-python-test-cpu.sh
@@ -0,0 +1,35 @@
+#!/usr/bin/env bash
+
+# exit when any command fails
+set -e
+
+# keep track of the last executed command
+trap 'last_command=$current_command; current_command=$BASH_COMMAND' DEBUG
+# echo an error message before exiting
+trap 'echo "\"${last_command}\" command filed with exit code $?."' EXIT
+
+# Install dependencies for building
+$GITHUB_WORKSPACE/dev/install-build-deps-ubuntu.sh
+
+# Setup building envs
+source /opt/intel/oneapi/setvars.sh --ccl-configuration=cpu
+
+# Prepare lib resources
+cd $GITHUB_WORKSPACE/mllib-dal
+../dev/prepare-build-deps.sh
+./build.sh -p CPU_GPU_PROFILE -q
+
+# Setup cluster
+source $GITHUB_WORKSPACE/dev/test-cluster/standalone/setup-cluster.sh
+
+# Setup OAP MLlib envs
+cp $GITHUB_WORKSPACE/dev/test-cluster/standalone/env.sh $GITHUB_WORKSPACE/conf
+cd $GITHUB_WORKSPACE/examples
+
+
+echo "========================================="
+echo "Cluster Testing with Spark Version: $SPARK_VERSION"
+echo "========================================="
+
+# run all examples
+./run-all-pyspark-cpu.sh
diff --git a/dev/ci/ci-standalone-test-cpu.sh → dev/ci/ci-standalone-scala-test-cpu.sh b/dev/ci/ci-standalone-test-cpu.sh → dev/ci/ci-standalone-scala-test-cpu.sh
@@ -34,4 +34,3 @@ echo "========================================="
 # Build and run all examples
 ./build-all-scala.sh
 ./run-all-scala-cpu.sh
-./run-all-pyspark-cpu.sh
diff --git a/mllib-dal/src/main/java/com/intel/oap/mllib/CCLParam.java b/mllib-dal/src/main/java/com/intel/oap/mllib/CCLParam.java
@@ -17,6 +17,22 @@
 package com.intel.oap.mllib;
 
 class CCLParam {
-  long commSize;
-  long rankId;
+  private long commSize;
+  private long rankId;
+
+  public long getCommSize() {
+    return commSize;
+  }
+
+  public void setCommSize(long commSize) {
+    this.commSize = commSize;
+  }
+
+  public long getRankId() {
+    return rankId;
+  }
+
+  public void setRankId(long rankId) {
+    this.rankId = rankId;
+  }
 }
diff --git a/mllib-dal/src/main/java/com/intel/oap/mllib/LibLoader.java b/mllib-dal/src/main/java/com/intel/oap/mllib/LibLoader.java
@@ -135,7 +135,7 @@ private static File createTempFile(String tempSubDirName, String name) throws IO
             System.getProperty("java.io.tmpdir") + "/" + tempSubDirName + LIBRARY_PATH_IN_JAR);
 
     if (!tempSubDirectory.exists()) {
-      tempSubDirectory.mkdirs();
+      boolean created = tempSubDirectory.mkdirs();
       // Check existance again, don't use return bool of mkdirs
       if (!tempSubDirectory.exists()) {
         throw new IOException("Error: Can`t create folder for temp file.");

diff --git a/mllib-dal/src/main/java/com/intel/oap/mllib/classification/NaiveBayesResult.java b/mllib-dal/src/main/java/com/intel/oap/mllib/classification/NaiveBayesResult.java
@@ -1,6 +1,22 @@
 package com.intel.oap.mllib.classification;
 
 public class NaiveBayesResult {
-    public long piNumericTable;
-    public long thetaNumericTable;
+    private long piNumericTable;
+    private long thetaNumericTable;
+
+    public long getPiNumericTable() {
+        return piNumericTable;
+    }
+
+    public void setPiNumericTable(long piNumericTable) {
+        this.piNumericTable = piNumericTable;
+    }
+
+    public long getThetaNumericTable() {
+        return thetaNumericTable;
+    }
+
+    public void setThetaNumericTable(long thetaNumericTable) {
+        this.thetaNumericTable = thetaNumericTable;
+    }
 }
diff --git a/mllib-dal/src/main/java/com/intel/oap/mllib/classification/RandomForestResult.java b/mllib-dal/src/main/java/com/intel/oap/mllib/classification/RandomForestResult.java
@@ -1,7 +1,31 @@
 package com.intel.oap.mllib.classification;
 
 public class RandomForestResult {
-    public long predictionNumericTable;
-    public long probabilitiesNumericTable;
-    public long importancesNumericTable;
+    private long predictionNumericTable;
+    private long probabilitiesNumericTable;
+    private long importancesNumericTable;
+
+    public long getProbabilitiesNumericTable() {
+        return probabilitiesNumericTable;
+    }
+
+    public void setProbabilitiesNumericTable(long probabilitiesNumericTable) {
+        this.probabilitiesNumericTable = probabilitiesNumericTable;
+    }
+
+    public long getPredictionNumericTable() {
+        return predictionNumericTable;
+    }
+
+    public void setPredictionNumericTable(long predictionNumericTable) {
+        this.predictionNumericTable = predictionNumericTable;
+    }
+
+    public long getImportancesNumericTable() {
+        return importancesNumericTable;
+    }
+
+    public void setImportancesNumericTable(long importancesNumericTable) {
+        this.importancesNumericTable = importancesNumericTable;
+    }
 }
diff --git a/mllib-dal/src/main/java/com/intel/oap/mllib/clustering/KMeansResult.java b/mllib-dal/src/main/java/com/intel/oap/mllib/clustering/KMeansResult.java
@@ -17,6 +17,22 @@
 package com.intel.oap.mllib.clustering;
 
 public class KMeansResult {
-  public int iterationNum;
-  public double totalCost;
+  private int iterationNum;
+  private double totalCost;
+
+  public int getIterationNum() {
+    return iterationNum;
+  }
+
+  public void setIterationNum(int iterationNum) {
+    this.iterationNum = iterationNum;
+  }
+
+  public double getTotalCost() {
+    return totalCost;
+  }
+
+  public void setTotalCost(double totalCost) {
+    this.totalCost = totalCost;
+  }
 }
diff --git a/mllib-dal/src/main/java/com/intel/oap/mllib/feature/PCAResult.java b/mllib-dal/src/main/java/com/intel/oap/mllib/feature/PCAResult.java
@@ -17,6 +17,24 @@
 package com.intel.oap.mllib.feature;
 
 public class PCAResult {
-  public long pcNumericTable;
-  public long explainedVarianceNumericTable;
+  private long pcNumericTable;
+  private long explainedVarianceNumericTable;
+
+  public long getExplainedVarianceNumericTable() {
+    return explainedVarianceNumericTable;
+  }
+
+  public void setExplainedVarianceNumericTable(long explainedVarianceNumericTable) {
+    this.explainedVarianceNumericTable = explainedVarianceNumericTable;
+  }
+
+  public long getPcNumericTable() {
+    return pcNumericTable;
+  }
+
+  public void setPcNumericTable(long pcNumericTable) {
+    this.pcNumericTable = pcNumericTable;
+  }
+
+
 }
diff --git a/mllib-dal/src/main/java/com/intel/oap/mllib/recommendation/ALSPartitionInfo.java b/mllib-dal/src/main/java/com/intel/oap/mllib/recommendation/ALSPartitionInfo.java
@@ -17,6 +17,22 @@
 package com.intel.oap.mllib.recommendation;
 
 public class ALSPartitionInfo {
-  public int ratingsNum;
-  public int csrRowNum;
+  private int ratingsNum;
+  private int csrRowNum;
+
+  public int getRatingsNum() {
+    return ratingsNum;
+  }
+
+  public void setRatingsNum(int ratingsNum) {
+    this.ratingsNum = ratingsNum;
+  }
+
+  public int getCsrRowNum() {
+    return csrRowNum;
+  }
+
+  public void setCsrRowNum(int csrRowNum) {
+    this.csrRowNum = csrRowNum;
+  }
 }
diff --git a/mllib-dal/src/main/java/com/intel/oap/mllib/recommendation/ALSResult.java b/mllib-dal/src/main/java/com/intel/oap/mllib/recommendation/ALSResult.java
@@ -17,9 +17,49 @@
 package com.intel.oap.mllib.recommendation;
 
 public class ALSResult {
-  public long rankId = -1;
-  public long cUsersFactorsNumTab;
-  public long cItemsFactorsNumTab;
-  public long cUserOffset;
-  public long cItemOffset;
+  private long rankId = -1;
+  private long cUsersFactorsNumTab;
+  private long cItemsFactorsNumTab;
+  private long cUserOffset;
+  private long cItemOffset;
+
+  public long getRankId() {
+    return rankId;
+  }
+
+  public void setRankId(long rankId) {
+    this.rankId = rankId;
+  }
+
+  public long getcUsersFactorsNumTab() {
+    return cUsersFactorsNumTab;
+  }
+
+  public void setcUsersFactorsNumTab(long cUsersFactorsNumTab) {
+    this.cUsersFactorsNumTab = cUsersFactorsNumTab;
+  }
+
+  public long getcItemsFactorsNumTab() {
+    return cItemsFactorsNumTab;
+  }
+
+  public void setcItemsFactorsNumTab(long cItemsFactorsNumTab) {
+    this.cItemsFactorsNumTab = cItemsFactorsNumTab;
+  }
+
+  public long getcUserOffset() {
+    return cUserOffset;
+  }
+
+  public void setcUserOffset(long cUserOffset) {
+    this.cUserOffset = cUserOffset;
+  }
+
+  public long getcItemOffset() {
+    return cItemOffset;
+  }
+
+  public void setcItemOffset(long cItemOffset) {
+    this.cItemOffset = cItemOffset;
+  }
 }
diff --git a/mllib-dal/src/main/java/com/intel/oap/mllib/regression/LiRResult.java b/mllib-dal/src/main/java/com/intel/oap/mllib/regression/LiRResult.java
@@ -17,5 +17,13 @@
 package com.intel.oap.mllib.regression;
 
 public class LiRResult {
-    public long coeffNumericTable;    // first element of coeff is actually intercept
+    private long coeffNumericTable;    // first element of coeff is actually intercept
+
+    public long getCoeffNumericTable() {
+        return coeffNumericTable;
+    }
+
+    public void setCoeffNumericTable(long coeffNumericTable) {
+        this.coeffNumericTable = coeffNumericTable;
+    }
 }
diff --git a/mllib-dal/src/main/java/com/intel/oap/mllib/stat/CorrelationResult.java b/mllib-dal/src/main/java/com/intel/oap/mllib/stat/CorrelationResult.java
@@ -17,5 +17,13 @@
 package com.intel.oap.mllib.stat;
 
 public class CorrelationResult {
-    public long correlationNumericTable;
+    private long correlationNumericTable;
+
+    public long getCorrelationNumericTable() {
+        return correlationNumericTable;
+    }
+
+    public void setCorrelationNumericTable(long correlationNumericTable) {
+        this.correlationNumericTable = correlationNumericTable;
+    }
 }
diff --git a/mllib-dal/src/main/java/com/intel/oap/mllib/stat/SummarizerResult.java b/mllib-dal/src/main/java/com/intel/oap/mllib/stat/SummarizerResult.java
@@ -14,8 +14,40 @@
 package com.intel.oap.mllib.stat;
 
 public class SummarizerResult {
-  public long meanNumericTable;
-  public long varianceNumericTable;
-  public long minimumNumericTable;
-  public long maximumNumericTable;
+  private long meanNumericTable;
+  private long varianceNumericTable;
+  private long minimumNumericTable;
+  private long maximumNumericTable;
+
+  public long getMeanNumericTable() {
+    return meanNumericTable;
+  }
+
+  public void setMeanNumericTable(long meanNumericTable) {
+    this.meanNumericTable = meanNumericTable;
+  }
+
+  public long getVarianceNumericTable() {
+    return varianceNumericTable;
+  }
+
+  public void setVarianceNumericTable(long varianceNumericTable) {
+    this.varianceNumericTable = varianceNumericTable;
+  }
+
+  public long getMinimumNumericTable() {
+    return minimumNumericTable;
+  }
+
+  public void setMinimumNumericTable(long minimumNumericTable) {
+    this.minimumNumericTable = minimumNumericTable;
+  }
+
+  public long getMaximumNumericTable() {
+    return maximumNumericTable;
+  }
+
+  public void setMaximumNumericTable(long maximumNumericTable) {
+    this.maximumNumericTable = maximumNumericTable;
+  }
 }
diff --git a/mllib-dal/src/main/native/ALSShuffle.cpp b/mllib-dal/src/main/native/ALSShuffle.cpp
@@ -49,7 +49,7 @@ bool compareRatingUserEquality(Rating &r1, Rating &r2) {
 int distinct_count(std::vector<Rating> &data) {
     long curUser = -1;
     long count = 0;
-    for (auto i : data) {
+    for (auto &i : data) {
         if (i.user > curUser) {
             curUser = i.user;
             count += 1;