fixed OSPDT review issues

Signed-off-by: minmingzhu <minming.zhu@intel.com>
oap-project · Aug 1, 2023 · 8541510 · 8541510
1 parent e39a186
commit 8541510
Show file tree

Hide file tree

Showing 23 changed files with 58 additions and 77 deletions.
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,8 +17,8 @@
 package com.intel.oap.mllib;
 
 class CCLParam {
-  long commSize;
-  long rankId;
+  private long commSize;
+  private long rankId;
 
   public long getCommSize() {
     return commSize;

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,8 +1,8 @@
 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;

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,9 +1,9 @@
 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;

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,8 +17,8 @@
 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;

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,8 +17,8 @@
 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;

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,11 +17,11 @@
 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;

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,7 +17,7 @@
 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;

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,7 +17,7 @@
 package com.intel.oap.mllib.stat;
 
 public class CorrelationResult {
-    public long correlationNumericTable;
+    private long correlationNumericTable;
 
     public long getCorrelationNumericTable() {
         return 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,10 +14,10 @@
 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;

diff --git a/mllib-dal/src/main/native/OneCCL.cpp b/mllib-dal/src/main/native/OneCCL.cpp
@@ -147,7 +147,7 @@ static int fill_local_host_ip() {
             continue;
         if (strstr(ifa->ifa_name, iface_name) == NULL) {
             family = ifa->ifa_addr->sa_family;
-            if (family == AF_INET || family == AF_INET6) {
+            if (family == AF_INET) {
                 memset(local_ip, 0, CCL_IP_LEN);
                 int res = getnameinfo(
                     ifa->ifa_addr,
@@ -161,11 +161,6 @@ static int fill_local_host_ip() {
                     return -1;
                 }
                 local_host_ips.push_back(local_ip);
-            } else {
-                std::cerr
-                    << "OneCCL (native): can't find interface to get host IP"
-                    << std::endl;
-                return -1;
             }
         }
     }

diff --git a/mllib-dal/src/main/native/Profile.hpp b/mllib-dal/src/main/native/Profile.hpp
@@ -6,10 +6,10 @@
 
 class Profiler {
   public:
-    Profiler(std::string s) : subject(std::move(s)) {}
+    Profiler(const std::string &s) : subject(s) {}
 
     void startProfile(std::string s = "") {
-        action = std::move(s);
+        action = s;
         std::cout << subject << " (native): start " << action << std::endl;
         startTime = std::chrono::high_resolution_clock::now();
     }

diff --git a/mllib-dal/src/main/native/oneapi/dal/ColumnAccessorImpl.cpp b/mllib-dal/src/main/native/oneapi/dal/ColumnAccessorImpl.cpp
@@ -44,7 +44,7 @@ JNIEXPORT jdoubleArray JNICALL Java_com_intel_oneapi_dal_table_ColumnAccessor_cP
   homogen_table htable = *reinterpret_cast<homogen_table *>(cTableAddr);
   column_accessor<const double> acc{ htable };
   oneapi::dal::array<double> col_values;
-  jdoubleArray newDoubleArray;
+  jdoubleArray newDoubleArray = nullptr;
   ComputeDevice device = getComputeDeviceByOrdinal(computeDeviceOrdinal);
   switch(device) {
        case ComputeDevice::host:{
@@ -78,7 +78,7 @@ JNIEXPORT jfloatArray JNICALL Java_com_intel_oneapi_dal_table_ColumnAccessor_cPu
   homogen_table htable = *reinterpret_cast<homogen_table *>(cTableAddr);
   column_accessor<const float> acc{ htable };
   oneapi::dal::array<float> col_values;
-  jfloatArray newFloatArray;
+  jfloatArray newFloatArray = nullptr;
   ComputeDevice device = getComputeDeviceByOrdinal(computeDeviceOrdinal);
   switch(device) {
        case ComputeDevice::host:{
@@ -112,7 +112,7 @@ JNIEXPORT jintArray JNICALL Java_com_intel_oneapi_dal_table_ColumnAccessor_cPull
     homogen_table htable = *reinterpret_cast<homogen_table *>(cTableAddr);
     column_accessor<const int> acc { htable };
     oneapi::dal::array<int> col_values;
-    jintArray newIntArray;
+    jintArray newIntArray = nullptr;
     ComputeDevice device = getComputeDeviceByOrdinal(computeDeviceOrdinal);
 switch(device) {
      case ComputeDevice::host:{

diff --git a/mllib-dal/src/main/native/oneapi/dal/RowAccessorImpl.cpp b/mllib-dal/src/main/native/oneapi/dal/RowAccessorImpl.cpp
@@ -44,7 +44,7 @@ JNIEXPORT jdoubleArray JNICALL Java_com_intel_oneapi_dal_table_RowAccessor_cPull
   homogen_table htable = *reinterpret_cast<const homogen_table *>(cTableAddr);
   row_accessor<const double> acc {htable};
   jdoubleArray newDoubleArray;
-  oneapi::dal::array<double> row_values;
+  oneapi::dal::array<double> row_values = nullptr;
   ComputeDevice device = getComputeDeviceByOrdinal(computeDeviceOrdinal);
   switch(device) {
          case ComputeDevice::host:{
@@ -78,7 +78,7 @@ JNIEXPORT jfloatArray JNICALL Java_com_intel_oneapi_dal_table_RowAccessor_cPullF
   homogen_table htable = *reinterpret_cast<const homogen_table *>(cTableAddr);
   row_accessor<const float> acc { htable };
   jfloatArray newFloatArray;
-  oneapi::dal::array<float> row_values;
+  oneapi::dal::array<float> row_values = nullptr;
   ComputeDevice device = getComputeDeviceByOrdinal(computeDeviceOrdinal);
   switch(device) {
          case ComputeDevice::host:{
@@ -112,7 +112,7 @@ JNIEXPORT jintArray JNICALL Java_com_intel_oneapi_dal_table_RowAccessor_cPullInt
   homogen_table htable = *reinterpret_cast<homogen_table *>(cTableAddr);
   row_accessor<const int> acc { htable };
   jintArray newIntArray;
-  oneapi::dal::array<int> row_values;
+  oneapi::dal::array<int> row_values = nullptr;
   ComputeDevice device = getComputeDeviceByOrdinal(computeDeviceOrdinal);
   switch(device) {
          case ComputeDevice::host:{

diff --git a/mllib-dal/src/main/native/service.cpp b/mllib-dal/src/main/native/service.cpp
@@ -446,10 +446,6 @@ void printNumericTable(const NumericTablePtr &dataTable, const char *message,
 
 void printPackedNumericTable(NumericTable *dataTable, size_t nFeatures,
                              const char *message = "", size_t interval = 10) {
-    // Save the original format state of std::cout
-    std::streamsize originalPrecision = std::cout.precision();
-    std::ios_base::fmtflags originalFlags = std::cout.flags();
-
     BlockDescriptor<DAAL_DATA_TYPE> block;
 
     dataTable->getBlockOfRows(0, 1, readOnly, block);
@@ -470,10 +466,6 @@ void printPackedNumericTable(NumericTable *dataTable, size_t nFeatures,
     }
     std::cout << std::endl;
 
-    // Restore the original format state of std::cout
-    std::cout.precision(originalPrecision);
-    std::cout.flags(originalFlags);
-
     dataTable->releaseBlockOfRows(block);
 }
 
@@ -542,10 +534,6 @@ void printNumericTables(NumericTable *dataTable1, NumericTable *dataTable2,
                         const char *title1 = "", const char *title2 = "",
                         const char *message = "", size_t nPrintedRows = 0,
                         size_t interval = 10) {
-    // Save the original format state of std::cout
-    std::streamsize originalPrecision = std::cout.precision();
-    std::ios_base::fmtflags originalFlags = std::cout.flags();
-
     size_t nRows1 = dataTable1->getNumberOfRows();
     size_t nRows2 = dataTable2->getNumberOfRows();
     size_t nCols1 = dataTable1->getNumberOfColumns();
@@ -584,10 +572,6 @@ void printNumericTables(NumericTable *dataTable1, NumericTable *dataTable2,
     }
     std::cout << std::endl;
 
-    // Restore the original format state of std::cout
-    std::cout.precision(originalPrecision);
-    std::cout.flags(originalFlags);
-
     dataTable1->releaseBlockOfRows(block1);
     dataTable2->releaseBlockOfRows(block2);
 }

diff --git a/mllib-dal/src/main/scala/com/intel/oap/mllib/OneCCL.scala b/mllib-dal/src/main/scala/com/intel/oap/mllib/OneCCL.scala
@@ -34,10 +34,11 @@ object OneCCL extends Logging {
     c_init(executor_num, rank, ip_port, cclParam)
 
     // executor number should equal to oneCCL world size
-    assert(executor_num == cclParam.commSize, "executor number should equal to oneCCL world size")
+    assert(executor_num == cclParam.getCommSize,
+      "executor number should equal to oneCCL world size")
 
     logInfo(s"Initialized with executorNum: $executor_num, " +
-      s"commSize, ${cclParam.commSize}, rankId: ${cclParam.rankId}")
+      s"commSize, ${cclParam.getCommSize}, rankId: ${cclParam.getRankId}")
   }
 
   // Run on Executor

diff --git a/mllib-dal/src/main/scala/com/intel/oap/mllib/classification/NaiveBayesDALImpl.scala b/mllib-dal/src/main/scala/com/intel/oap/mllib/classification/NaiveBayesDALImpl.scala
@@ -71,8 +71,9 @@ class NaiveBayesDALImpl(val uid: String,
         val ret = if (OneCCL.isRoot()) {
           val convResultStartTime = System.nanoTime()
 
-          val pi = OneDAL.numericTableNx1ToVector(OneDAL.makeNumericTable(result.piNumericTable))
-          val theta = OneDAL.numericTableToMatrix(OneDAL.makeNumericTable(result.thetaNumericTable))
+          val pi = OneDAL.numericTableNx1ToVector(OneDAL.makeNumericTable(result.getPiNumericTable))
+          val theta = OneDAL.numericTableToMatrix(
+            OneDAL.makeNumericTable(result.getThetaNumericTable))
 
           val convResultEndTime = System.nanoTime()
 

diff --git a/mllib-dal/src/main/scala/com/intel/oap/mllib/clustering/KMeansDALImpl.scala b/mllib-dal/src/main/scala/com/intel/oap/mllib/clustering/KMeansDALImpl.scala
@@ -89,7 +89,7 @@ class KMeansDALImpl(var nClusters: Int,
           } else {
             OneDAL.numericTableToVectors(OneDAL.makeNumericTable(cCentroids))
           }
-          Iterator((centerVectors, result.totalCost, result.iterationNum))
+          Iterator((centerVectors, result.getTotalCost, result.getIterationNum))
         } else {
           Iterator.empty
         }

diff --git a/mllib-dal/src/main/scala/com/intel/oap/mllib/feature/PCADALImpl.scala b/mllib-dal/src/main/scala/com/intel/oap/mllib/feature/PCADALImpl.scala
@@ -80,21 +80,21 @@ class PCADALImpl(val k: Int,
 
       val ret = if (rank == 0) {
         val principleComponents = if (useDevice == "GPU") {
-          val pcNumericTable = OneDAL.makeHomogenTable(result.pcNumericTable)
+          val pcNumericTable = OneDAL.makeHomogenTable(result.getPcNumericTable)
           getPrincipleComponentsFromOneAPI(pcNumericTable, k, computeDevice)
         } else {
-          val pcNumericTable = OneDAL.makeNumericTable(result.pcNumericTable)
+          val pcNumericTable = OneDAL.makeNumericTable(result.getPcNumericTable)
           getPrincipleComponentsFromDAL(pcNumericTable, k)
         }
 
         val explainedVariance = if (useDevice == "GPU") {
           val explainedVarianceNumericTable = OneDAL.makeHomogenTable(
-            result.explainedVarianceNumericTable)
+            result.getExplainedVarianceNumericTable)
           getExplainedVarianceFromOneAPI(
             explainedVarianceNumericTable, k, computeDevice)
         } else {
           val explainedVarianceNumericTable = OneDAL.makeNumericTable(
-            result.explainedVarianceNumericTable)
+            result.getExplainedVarianceNumericTable)
           getExplainedVarianceFromDAL(explainedVarianceNumericTable, k)
         }
 

diff --git a/mllib-dal/src/main/scala/com/intel/oap/mllib/recommendation/ALSDALImpl.scala b/mllib-dal/src/main/scala/com/intel/oap/mllib/recommendation/ALSDALImpl.scala
@@ -111,8 +111,8 @@ class ALSDALImpl[@specialized(Int, Long) ID: ClassTag]( data: RDD[Rating[ID]],
     val usersFactorsRDD = results
       .mapPartitionsWithIndex { (index: Int, partiton: Iterator[ALSResult]) =>
         val ret = partiton.flatMap { p =>
-          val userOffset = p.cUserOffset.toInt
-          val usersFactorsNumTab = OneDAL.makeNumericTable(p.cUsersFactorsNumTab)
+          val userOffset = p.getcUserOffset().toInt
+          val usersFactorsNumTab = OneDAL.makeNumericTable(p.getcUsersFactorsNumTab())
           val nRows = usersFactorsNumTab.getNumberOfRows.toInt
           val nCols = usersFactorsNumTab.getNumberOfColumns.toInt
           var buffer = FloatBuffer.allocate(nCols * nRows)
@@ -132,8 +132,8 @@ class ALSDALImpl[@specialized(Int, Long) ID: ClassTag]( data: RDD[Rating[ID]],
     val itemsFactorsRDD = results
       .mapPartitionsWithIndex { (index: Int, partiton: Iterator[ALSResult]) =>
         val ret = partiton.flatMap { p =>
-          val itemOffset = p.cItemOffset.toInt
-          val itemsFactorsNumTab = OneDAL.makeNumericTable(p.cItemsFactorsNumTab)
+          val itemOffset = p.getcItemOffset().toInt
+          val itemsFactorsNumTab = OneDAL.makeNumericTable(p.getcItemsFactorsNumTab())
           val nRows = itemsFactorsNumTab.getNumberOfRows.toInt
           val nCols = itemsFactorsNumTab.getNumberOfColumns.toInt
           var buffer = FloatBuffer.allocate(nCols * nRows)

diff --git a/mllib-dal/src/main/scala/com/intel/oap/mllib/regression/LinearRegressionDALImpl.scala b/mllib-dal/src/main/scala/com/intel/oap/mllib/regression/LinearRegressionDALImpl.scala
@@ -99,7 +99,7 @@ class LinearRegressionDALImpl( val fitIntercept: Boolean,
     }
 
     // OAP MLlib: Only normal linear regression is supported for GPU currently
-    if (useDevice == "GPU" && regParam != 0){
+    if (useDevice == "GPU" && regParam != 0) {
       val msg = s"OAP MLlib: Regularization parameter is not supported for GPU now."
       logError(msg)
       throw new SparkException(msg)

diff --git a/mllib-dal/src/main/scala/com/intel/oap/mllib/regression/RandomForestRegressorDALImpl.scala b/mllib-dal/src/main/scala/com/intel/oap/mllib/regression/RandomForestRegressorDALImpl.scala
@@ -112,7 +112,7 @@ class RandomForestRegressorDALImpl(val uid: String,
       val ret = if (rank == 0) {
         val convResultStartTime = System.nanoTime()
         val predictionNumericTable = OneDAL.homogenTableToMatrix(
-          OneDAL.makeHomogenTable(result.predictionNumericTable),
+          OneDAL.makeHomogenTable(result.getPredictionNumericTable),
           computeDevice)
         val convResultEndTime = System.nanoTime()
 

diff --git a/mllib-dal/src/main/scala/com/intel/oap/mllib/stat/CorrelationDALImpl.scala b/mllib-dal/src/main/scala/com/intel/oap/mllib/stat/CorrelationDALImpl.scala
@@ -77,10 +77,10 @@ class CorrelationDALImpl(
       val ret = if (rank == 0) {
         val convResultStartTime = System.nanoTime()
         val correlationNumericTable = if (useDevice == "GPU") {
-          OneDAL.homogenTableToMatrix(OneDAL.makeHomogenTable(result.correlationNumericTable),
+          OneDAL.homogenTableToMatrix(OneDAL.makeHomogenTable(result.getCorrelationNumericTable),
             computeDevice)
         } else {
-          OneDAL.numericTableToMatrix(OneDAL.makeNumericTable(result.correlationNumericTable))
+          OneDAL.numericTableToMatrix(OneDAL.makeNumericTable(result.getCorrelationNumericTable))
         }
         val convResultEndTime = System.nanoTime()
 

diff --git a/mllib-dal/src/main/scala/com/intel/oap/mllib/stat/SummarizerDALImpl.scala b/mllib-dal/src/main/scala/com/intel/oap/mllib/stat/SummarizerDALImpl.scala
@@ -80,31 +80,31 @@ class SummarizerDALImpl(val executorNum: Int,
         val convResultStartTime = System.nanoTime()
         val meanVector = if (useDevice == "GPU") {
           OneDAL.homogenTable1xNToVector(
-            OneDAL.makeHomogenTable(result.meanNumericTable), computeDevice)
+            OneDAL.makeHomogenTable(result.getMeanNumericTable), computeDevice)
         } else {
           OneDAL.numericTable1xNToVector(
-            OneDAL.makeNumericTable(result.meanNumericTable))
+            OneDAL.makeNumericTable(result.getMeanNumericTable))
         }
         val varianceVector = if (useDevice == "GPU") {
           OneDAL.homogenTable1xNToVector(
-            OneDAL.makeHomogenTable(result.varianceNumericTable), computeDevice)
+            OneDAL.makeHomogenTable(result.getVarianceNumericTable), computeDevice)
         } else {
           OneDAL.numericTable1xNToVector(
-            OneDAL.makeNumericTable(result.varianceNumericTable))
+            OneDAL.makeNumericTable(result.getVarianceNumericTable))
         }
         val maxVector = if (useDevice == "GPU") {
           OneDAL.homogenTable1xNToVector(
-            OneDAL.makeHomogenTable(result.maximumNumericTable), computeDevice)
+            OneDAL.makeHomogenTable(result.getMaximumNumericTable), computeDevice)
         } else {
           OneDAL.numericTable1xNToVector(
-            OneDAL.makeNumericTable(result.maximumNumericTable))
+            OneDAL.makeNumericTable(result.getMaximumNumericTable))
         }
         val minVector = if (useDevice == "GPU") {
           OneDAL.homogenTable1xNToVector(
-            OneDAL.makeHomogenTable(result.minimumNumericTable), computeDevice)
+            OneDAL.makeHomogenTable(result.getMinimumNumericTable), computeDevice)
         } else {
           OneDAL.numericTable1xNToVector(
-            OneDAL.makeNumericTable(result.minimumNumericTable))
+            OneDAL.makeNumericTable(result.getMinimumNumericTable))
         }
 
         val convResultEndTime = System.nanoTime()