Merge pull request #32 from scikit-time/use_cxx14_osx

Use only cxx14 features in clustering extension.

CMakeLists.txt

@@ -1,6 +1,6 @@
 cmake_minimum_required(VERSION 3.9)
 
-set(CMAKE_CXX_STANDARD 17)
+set(CMAKE_CXX_STANDARD 14)
 
 if(DEFINED PROJECT_NAME)
     set(SKTIME_IS_SUBPROJECT ON)

azure-pipelines.yml

@@ -2,7 +2,6 @@ stages:
 - stage: Test
   jobs:
   - template: ./devtools/azure-pipelines-linux.yml
-  # TODO: enable 
-  #- template: ./devtools/azure-pipelines-osx.yml
+  - template: ./devtools/azure-pipelines-osx.yml
   #- template: ./devtools/azure-pipelines-win.yml
 

devtools/azure-pipelines-linux.yml

@@ -15,6 +15,7 @@ jobs:
     maxParallel: 10
 
   steps:
+  - template: checkout.yml
   - bash: echo "##vso[task.prependpath]$CONDA/bin"
     displayName: Add conda to PATH
 

devtools/azure-pipelines-osx.yml

@@ -10,11 +10,13 @@ jobs:
         CONDA_NPY: '1.17'
 
   steps:
+  - template: checkout.yml
   - bash: echo "##vso[task.prependpath]$CONDA/bin"
     displayName: Add conda to PATH
 
   - script: sudo chmod -R 777 /usr/local/miniconda
     displayName: Fix Conda permissions
+  - script: env
 
   - template: conda-setup+build.yml
   - template: test-results.yml

devtools/checkout.yml

@@ -0,0 +1,4 @@
+steps:
+- checkout: self
+  submodules: true
+

devtools/conda-recipe/meta.yaml

@@ -8,8 +8,8 @@ source:
 
 build:
   script_env:
-   - CIRCLE_TEST_REPORTS
    - OMP_NUM_THREADS
+   - AGENT_BUILDDIRECTORY
   script: "{{ PYTHON }} -m pip install . --no-deps --ignore-installed --no-cache-dir -vvv"
 
 requirements:
@@ -22,13 +22,11 @@ requirements:
   host:
     - cython
     - intel-openmp # [osx]
-    - mdtraj
     - numpy
     - python
     - scipy
     - setuptools
     - pip
-    - pybind11
 
   run:
     #- bhmm >=0.6.3,<0.7

devtools/conda-recipe/run_test.py

@@ -2,17 +2,16 @@
 import sys
 
 import pytest
+import tempfile
 
 cover_pkg = 'sktime'
-
+xml_results_dest = os.getenv('SYSTEM_DEFAULTWORKINGDIRECTORY', tempfile.gettempdir())
+assert os.path.isdir(xml_results_dest), 'no dest dir available'
 # where to write junit xml
-junit_xml = os.path.join(os.getenv('CIRCLE_TEST_REPORTS', os.path.expanduser('~')),
-                         'reports', 'junit.xml')
-target_dir = os.path.dirname(junit_xml)
-if not os.path.exists(target_dir):
-    os.makedirs(target_dir)
+junit_xml = os.path.join(xml_results_dest, 'junit.xml')
+cov_xml = os.path.join(xml_results_dest, 'coverage.xml')
 print('junit destination:', junit_xml)
-
+print('coverage dest:', cov_xml)
 pytest_args = ("-vv "
                "--cov={cover_pkg} "
                "--cov-report=xml:{dest_report} "
@@ -22,7 +21,7 @@
                "tests/ "
                .format(cover_pkg=cover_pkg,
                        junit_xml=junit_xml,
-                       dest_report=os.path.join(os.path.expanduser('~/'), 'coverage.xml'),
+                       dest_report=cov_xml,
                        )
                .split(' '))
 

devtools/conda-setup+build.yml

@@ -3,7 +3,7 @@ steps:
       conda config --add channels conda-forge
       conda config --set always_yes true
       conda config --set quiet true
-      conda install conda-build
+      conda install conda-build conda-verify
     displayName: 'Install dependencies'
     continueOnError: false
   - bash: |

devtools/test-results.yml

@@ -3,7 +3,17 @@ steps:
 - task: PublishTestResults@2
   inputs:
     testResultsFormat: 'JUnit' # Options: JUnit, NUnit, VSTest, xUnit, cTest
-    testResultsFiles: '**/junit.xml'
-    searchFolder: '$(HOME)' # Optional
+    testResultsFiles: '**/*junit.xml'
     failTaskOnFailedTests: true # Optional
     publishRunAttachments: true # Optional
+
+- task: PublishCodeCoverageResults@1
+  inputs:
+    codeCoverageTool: 'cobertura' # Options: cobertura, jaCoCo
+    summaryFileLocation: '$(System.DefaultWorkingDirectory)/coverage.xml'
+    pathToSources: '$(Build.SourcesDirectory)/sktime' # Optional
+    #reportDirectory: # Optional
+    #additionalCodeCoverageFiles: # Optional
+    #failIfCoverageEmpty: false # Optional
+
+- bash: curl -s https://codecov.io/bash | bash

setup.py

@@ -70,9 +70,12 @@ def build_extension(self, ext):
 
         super(Build, self).build_extension(ext)
 
+
 cmdclass = versioneer.get_cmdclass()
 cmdclass['build_ext'] = Build
 
+# TODO: this is platform dependent, e.g. win should be treated differently.
+cxx_flags = ['-std=c++14']
 metadata = \
     dict(
         name='scikit-time',
@@ -84,18 +87,18 @@ def build_extension(self, ext):
         ext_modules=[
             Extension('sktime.covariance.util.covar_c._covartools', sources=[
                 'sktime/covariance/util/covar_c/covartools.cpp',
-            ], language='c++'),
+            ], language='c++', extra_compile_args=cxx_flags),
             Extension('sktime.numeric.eig_qr', sources=[
                 'sktime/numeric/eig_qr.pyx'],
                       language_level=3),
             Extension('sktime.clustering._clustering_bindings', sources=[
                 'sktime/clustering/src/clustering_module.cpp'
             ], include_dirs=['sktime/clustering/include'],
-                      language='c++', extra_compile_args=['-std=c++17']),
+                      language='c++', extra_compile_args=cxx_flags),
             Extension('sktime.markovprocess._markovprocess_bindings', sources=[
                 'sktime/markovprocess/src/markovprocess_module.cpp'
             ], include_dirs=['sktime/markovprocess/include'],
-                      language='c++', extra_compile_args=['-std=c++17']),
+                      language='c++', extra_compile_args=cxx_flags),
         ],
         cmdclass=cmdclass,
         zip_safe=False,
@@ -106,11 +109,8 @@ def build_extension(self, ext):
         },
     )
 
-# workaround for https://reviews.llvm.org/D8467, see https://github.com/pybind/pybind11/issues/1818
-if sys.platform == 'darwin':
-   for e in metadata['ext_modules']:
-       e.extra_compile_args.append('-fno-sized-deallocation')
-       e.extra_compile_args.append('-fno-aligned-allocation')
-
 if __name__ == '__main__':
+    import os
+    assert os.listdir(os.path.join('lib', 'pybind11')), 'ensure pybind11 submodule is initialized'
+
     setup(**metadata)

sktime/clustering/include/bits/kmeans_bits.h

@@ -14,7 +14,8 @@
 #include <pybind11/pytypes.h>
 #include <mutex>
 
-namespace clustering::kmeans {
+namespace clustering {
+namespace kmeans {
 
 template<typename T>
 inline np_array<T> cluster(const np_array<T> &np_chunk, const np_array<T> &np_centers, int n_threads,
@@ -411,3 +412,4 @@ inline np_array<T> initCentersKMpp(const np_array<T>& np_data, std::size_t k,
 }
 
 }
+}

sktime/clustering/include/bits/metric_base_bits.h

@@ -10,6 +10,11 @@
 #include <omp.h>
 #endif
 
+template<>
+inline float Metric::compute<float>(const float* xs, const float* ys, std::size_t dim) const {
+    return this->compute_f(xs, ys, dim);
+}
+
 template<typename T>
 inline py::array_t<int> assign_chunk_to_centers(const np_array<T>& chunk,
                                                 const np_array<T>& centers,

sktime/clustering/include/kmeans.h

@@ -9,7 +9,8 @@
 #include "common.h"
 #include "metric.h"
 
-namespace clustering::kmeans {
+namespace clustering {
+namespace kmeans {
 
 template<typename T>
 np_array<T> cluster(const np_array<T> & /*np_chunk*/, const np_array<T> & /*np_centers*/, int /*n_threads*/,
@@ -27,5 +28,5 @@ np_array<T> initCentersKMpp(const np_array<T>& np_data, std::size_t k, const Met
                             unsigned int random_seed, int n_threads, py::object& callback);
 
 }
-
+}
 #include "bits/kmeans_bits.h"

sktime/clustering/include/metric.h

@@ -16,16 +16,13 @@ namespace py = pybind11;
 
 class Metric {
 public:
+    virtual ~Metric() = default;
     virtual double compute_d(const double* xs, const double* ys, std::size_t dim) const = 0;
     virtual float compute_f(const float* xs, const float* ys, std::size_t dim) const = 0;
 
     template<typename T>
     T compute(const T* xs, const T* ys, std::size_t dim) const {
-        if constexpr (std::is_same_v<T, float>) {
-            return compute_f(xs, ys, dim);
-        } else {
-            return compute_d(xs, ys, dim);
-        }
+        return compute_d(xs, ys, dim);
     }
 };
 

sktime/clustering/include/regspace.h

@@ -11,7 +11,8 @@
 #include <omp.h>
 #endif
 
-namespace clustering::regspace {
+namespace clustering {
+namespace regspace {
 
 class MaxCentersReachedException : public std::exception {
 public:
@@ -70,3 +71,4 @@ void cluster(const np_array<T> &chunk, py::list& py_centers, T dmin, std::size_t
     }
 }
 }
+}

sktime/clustering/src/clustering_module.cpp

@@ -12,7 +12,11 @@ static const auto euclidean = EuclideanMetric{};
 
 template<typename T>
 std::tuple<py::object, int, int, double> castLoopResult(const std::tuple<np_array<T>, int, int, T> &input) {
-    const auto& [arr, res, it, cost] = input;
+    const auto& arr = std::get<0>(input);
+    const auto& res = std::get<1>(input);
+    const auto& it =  std::get<2>(input);
+    const auto& cost = std::get<3>(input);
+
     return std::make_tuple(py::cast<py::object>(arr), res, it, static_cast<double>(cost));
 }