From 022512d5abe6fd9d905d6fec855060e6a2930174 Mon Sep 17 00:00:00 2001 From: Scott Inglis Date: Tue, 5 Mar 2019 14:18:33 -0800 Subject: [PATCH 01/14] Updating ml.net to use the mlnetdep 0.0.0.9 nuget package. --- build/Dependencies.props | 2 +- .../Microsoft.ML.Console.csproj | 5 ++++- src/Native/SymSgdNative/CMakeLists.txt | 19 +++++++++++++++++-- src/Native/SymSgdNative/SymSgdNative.cpp | 10 +--------- src/Native/build.proj | 5 ++++- .../Microsoft.ML.Benchmarks.csproj | 3 ++- .../Microsoft.ML.Core.Tests.csproj | 3 ++- .../Microsoft.ML.Functional.Tests.csproj | 1 + .../Microsoft.ML.OnnxTransformerTest.csproj | 1 + .../Microsoft.ML.Predictor.Tests.csproj | 1 + .../Microsoft.ML.StaticPipelineTesting.csproj | 1 + .../Microsoft.ML.Sweeper.Tests.csproj | 1 + .../Microsoft.ML.TestFramework.csproj | 3 ++- .../Microsoft.ML.Tests.csproj | 1 + .../Microsoft.ML.TimeSeries.Tests.csproj | 3 ++- 15 files changed, 41 insertions(+), 18 deletions(-) diff --git a/build/Dependencies.props b/build/Dependencies.props index d2a48f6865..1d13bf6e65 100644 --- a/build/Dependencies.props +++ b/build/Dependencies.props @@ -15,7 +15,7 @@ 3.5.1 2.2.3 0.2.1 - 0.0.0.7 + 0.0.0.9 2.1.3 4.5.0 4.5.0 diff --git a/src/Microsoft.ML.Console/Microsoft.ML.Console.csproj b/src/Microsoft.ML.Console/Microsoft.ML.Console.csproj index a3b41c1265..a6db82a030 100644 --- a/src/Microsoft.ML.Console/Microsoft.ML.Console.csproj +++ b/src/Microsoft.ML.Console/Microsoft.ML.Console.csproj @@ -31,8 +31,11 @@ + - + + + diff --git a/src/Native/SymSgdNative/CMakeLists.txt b/src/Native/SymSgdNative/CMakeLists.txt index 4bbf9f69d6..a52eea443c 100644 --- a/src/Native/SymSgdNative/CMakeLists.txt +++ b/src/Native/SymSgdNative/CMakeLists.txt @@ -1,10 +1,25 @@ project (SymSgdNative) + set(SOURCES SymSgdNative.cpp ) +find_package(OpenMP) +if (OPENMP_FOUND) + if(APPLE) + SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Xpreprocessor -fopenmp") + SET(OPENMP_LIBRARY "omp") + include_directories("/usr/local/opt/libomp/include") + link_directories("/usr/local/opt/libomp/lib") + else() + SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${OpenMP_CXX_FLAGS}") + SET(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${OpenMP_EXE_LINKER_FLAGS}") + endif() +endif() + find_library(MKL_LIBRARY MklImports HINTS ${MKL_LIB_PATH}) + if(NOT WIN32) list(APPEND SOURCES ${VERSION_FILE_PATH}) if(NOT APPLE) @@ -16,10 +31,10 @@ if(NOT WIN32) endif() add_library(SymSgdNative SHARED ${SOURCES} ${RESOURCES}) -target_link_libraries(SymSgdNative PUBLIC ${MKL_LIBRARY}) +target_link_libraries(SymSgdNative PUBLIC ${MKL_LIBRARY} PUBLIC ${OPENMP_LIBRARY}) if(APPLE) set_target_properties(SymSgdNative PROPERTIES INSTALL_RPATH "@loader_path;@loader_path/${MKL_LIB_RPATH}}") endif() -install_library_and_symbols (SymSgdNative) \ No newline at end of file +install_library_and_symbols (SymSgdNative) diff --git a/src/Native/SymSgdNative/SymSgdNative.cpp b/src/Native/SymSgdNative/SymSgdNative.cpp index 680bc80798..14eab5ca62 100644 --- a/src/Native/SymSgdNative/SymSgdNative.cpp +++ b/src/Native/SymSgdNative/SymSgdNative.cpp @@ -6,9 +6,7 @@ #include #include -#if defined(USE_OMP) #include -#endif #include #include #include "../Stdafx.h" @@ -240,7 +238,6 @@ void InitializeState(int totalNumInstances, int* instSizes, int** instIndices, f TuneNumLocIter(numLocIter, totalNumInstances, instSizes, numThreads); state->WeightScaling = 1.0f; -#if defined(USE_OMP) if (numThreads > 1) { state->PassIteration = 0; @@ -268,7 +265,6 @@ void InitializeState(int totalNumInstances, int* instSizes, int** instIndices, f // To make sure that MKL runs sequentially omp_set_num_threads(1); -#endif } float Loss(int instSize, int* instIndices, float* instValues, @@ -361,7 +357,6 @@ EXPORT_API(void) LearnAll(int totalNumInstances, int* instSizes, int** instIndic } } else { -#if defined(USE_OMP) // In parallel case... bool shouldRemap = !((std::unordered_map*)state->FreqFeatUnorderedMap)->empty(); SymSGD** learners = (SymSGD**)(state->Learners); @@ -448,7 +443,6 @@ EXPORT_API(void) LearnAll(int totalNumInstances, int* instSizes, int** instIndic } } state->TotalInstancesProcessed += numPasses*totalNumInstances; -#endif } } @@ -471,10 +465,8 @@ EXPORT_API(void) MapBackWeightVector(float* weightVector, SymSGDState* state) // Deallocation method EXPORT_API(void) DeallocateSequentially(SymSGDState* state) { -#if defined(USE_OMP) // To make sure that for the rest of MKL calls use parallelism omp_set_num_threads(omp_get_num_procs()); -#endif SymSGD** learners = (SymSGD**)(state->Learners); if (learners) @@ -486,4 +478,4 @@ EXPORT_API(void) DeallocateSequentially(SymSGDState* state) delete (std::unordered_map*)state->FreqFeatUnorderedMap; if (state->FreqFeatDirectMap) delete[] state->FreqFeatDirectMap; -} \ No newline at end of file +} diff --git a/src/Native/build.proj b/src/Native/build.proj index 0d12196c26..e6f5e9ea50 100644 --- a/src/Native/build.proj +++ b/src/Native/build.proj @@ -72,7 +72,10 @@ - + + + diff --git a/test/Microsoft.ML.Benchmarks/Microsoft.ML.Benchmarks.csproj b/test/Microsoft.ML.Benchmarks/Microsoft.ML.Benchmarks.csproj index 81516c3eb8..bc6ca82f90 100644 --- a/test/Microsoft.ML.Benchmarks/Microsoft.ML.Benchmarks.csproj +++ b/test/Microsoft.ML.Benchmarks/Microsoft.ML.Benchmarks.csproj @@ -24,5 +24,6 @@ + - \ No newline at end of file + diff --git a/test/Microsoft.ML.Core.Tests/Microsoft.ML.Core.Tests.csproj b/test/Microsoft.ML.Core.Tests/Microsoft.ML.Core.Tests.csproj index 927cbc77ee..5de5caa554 100644 --- a/test/Microsoft.ML.Core.Tests/Microsoft.ML.Core.Tests.csproj +++ b/test/Microsoft.ML.Core.Tests/Microsoft.ML.Core.Tests.csproj @@ -28,11 +28,12 @@ - + + diff --git a/test/Microsoft.ML.Functional.Tests/Microsoft.ML.Functional.Tests.csproj b/test/Microsoft.ML.Functional.Tests/Microsoft.ML.Functional.Tests.csproj index 6d5b61350d..d263de063f 100644 --- a/test/Microsoft.ML.Functional.Tests/Microsoft.ML.Functional.Tests.csproj +++ b/test/Microsoft.ML.Functional.Tests/Microsoft.ML.Functional.Tests.csproj @@ -38,6 +38,7 @@ + diff --git a/test/Microsoft.ML.OnnxTransformerTest/Microsoft.ML.OnnxTransformerTest.csproj b/test/Microsoft.ML.OnnxTransformerTest/Microsoft.ML.OnnxTransformerTest.csproj index db261434bb..6d11c627ee 100644 --- a/test/Microsoft.ML.OnnxTransformerTest/Microsoft.ML.OnnxTransformerTest.csproj +++ b/test/Microsoft.ML.OnnxTransformerTest/Microsoft.ML.OnnxTransformerTest.csproj @@ -30,6 +30,7 @@ + diff --git a/test/Microsoft.ML.Predictor.Tests/Microsoft.ML.Predictor.Tests.csproj b/test/Microsoft.ML.Predictor.Tests/Microsoft.ML.Predictor.Tests.csproj index 1717f87de8..6f3effd363 100644 --- a/test/Microsoft.ML.Predictor.Tests/Microsoft.ML.Predictor.Tests.csproj +++ b/test/Microsoft.ML.Predictor.Tests/Microsoft.ML.Predictor.Tests.csproj @@ -21,6 +21,7 @@ + diff --git a/test/Microsoft.ML.StaticPipelineTesting/Microsoft.ML.StaticPipelineTesting.csproj b/test/Microsoft.ML.StaticPipelineTesting/Microsoft.ML.StaticPipelineTesting.csproj index ae2a57565c..14a11a50c4 100644 --- a/test/Microsoft.ML.StaticPipelineTesting/Microsoft.ML.StaticPipelineTesting.csproj +++ b/test/Microsoft.ML.StaticPipelineTesting/Microsoft.ML.StaticPipelineTesting.csproj @@ -26,5 +26,6 @@ + diff --git a/test/Microsoft.ML.Sweeper.Tests/Microsoft.ML.Sweeper.Tests.csproj b/test/Microsoft.ML.Sweeper.Tests/Microsoft.ML.Sweeper.Tests.csproj index 1ebd86b042..c8415ff24e 100644 --- a/test/Microsoft.ML.Sweeper.Tests/Microsoft.ML.Sweeper.Tests.csproj +++ b/test/Microsoft.ML.Sweeper.Tests/Microsoft.ML.Sweeper.Tests.csproj @@ -10,5 +10,6 @@ + diff --git a/test/Microsoft.ML.TestFramework/Microsoft.ML.TestFramework.csproj b/test/Microsoft.ML.TestFramework/Microsoft.ML.TestFramework.csproj index d24892a1a8..c90479e826 100644 --- a/test/Microsoft.ML.TestFramework/Microsoft.ML.TestFramework.csproj +++ b/test/Microsoft.ML.TestFramework/Microsoft.ML.TestFramework.csproj @@ -19,9 +19,10 @@ - + + diff --git a/test/Microsoft.ML.Tests/Microsoft.ML.Tests.csproj b/test/Microsoft.ML.Tests/Microsoft.ML.Tests.csproj index 383e0aef24..1fb9d6a42b 100644 --- a/test/Microsoft.ML.Tests/Microsoft.ML.Tests.csproj +++ b/test/Microsoft.ML.Tests/Microsoft.ML.Tests.csproj @@ -38,6 +38,7 @@ + diff --git a/test/Microsoft.ML.TimeSeries.Tests/Microsoft.ML.TimeSeries.Tests.csproj b/test/Microsoft.ML.TimeSeries.Tests/Microsoft.ML.TimeSeries.Tests.csproj index 9b23e12fc9..218978ae86 100644 --- a/test/Microsoft.ML.TimeSeries.Tests/Microsoft.ML.TimeSeries.Tests.csproj +++ b/test/Microsoft.ML.TimeSeries.Tests/Microsoft.ML.TimeSeries.Tests.csproj @@ -13,5 +13,6 @@ + - \ No newline at end of file + From 39c891556685bacbc25f6571ecb8acefb6db541e Mon Sep 17 00:00:00 2001 From: Scott Inglis Date: Tue, 5 Mar 2019 16:24:58 -0800 Subject: [PATCH 02/14] - Enabled NumberOfThreads parameter --- .../SymSgdClassificationTrainer.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Microsoft.ML.Mkl.Components/SymSgdClassificationTrainer.cs b/src/Microsoft.ML.Mkl.Components/SymSgdClassificationTrainer.cs index 8af12c405d..628989fe18 100644 --- a/src/Microsoft.ML.Mkl.Components/SymSgdClassificationTrainer.cs +++ b/src/Microsoft.ML.Mkl.Components/SymSgdClassificationTrainer.cs @@ -663,6 +663,10 @@ private TPredictor TrainCore(IChannel ch, RoleMappedData data, LinearModelParame int numFeatures = data.Schema.Feature.Value.Type.GetVectorSize(); var cursorFactory = new FloatLabelCursor.Factory(data, CursOpt.Label | CursOpt.Features); int numThreads = 1; + + if (_options.NumberOfThreads.HasValue) + numThreads = _options.NumberOfThreads.Value; + ch.CheckUserArg(numThreads > 0, nameof(_options.NumberOfThreads), "The number of threads must be either null or a positive integer."); From 63ca59a84155fb5a528a8ab2e285531fd15490ef Mon Sep 17 00:00:00 2001 From: Scott Inglis Date: Tue, 5 Mar 2019 20:55:59 -0800 Subject: [PATCH 03/14] - Update for mac build --- src/Native/SymSgdNative/CMakeLists.txt | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/Native/SymSgdNative/CMakeLists.txt b/src/Native/SymSgdNative/CMakeLists.txt index a52eea443c..35c80bc5a6 100644 --- a/src/Native/SymSgdNative/CMakeLists.txt +++ b/src/Native/SymSgdNative/CMakeLists.txt @@ -5,14 +5,19 @@ set(SOURCES SymSgdNative.cpp ) -find_package(OpenMP) -if (OPENMP_FOUND) - if(APPLE) - SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Xpreprocessor -fopenmp") - SET(OPENMP_LIBRARY "omp") - include_directories("/usr/local/opt/libomp/include") - link_directories("/usr/local/opt/libomp/lib") - else() +if(APPLE) + # CMake has support for OpenMP, however, Apple has a version of Clang + # that does not support openMP out of the box. Therefore + # these commands are added to sepcifically handle the Apple Clang scenario + # If the LLVM version of clang is used for Apple builds, this can be removed + # and the else condition can be used instead. + SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Xpreprocessor -fopenmp") + SET(OPENMP_LIBRARY "omp") + include_directories("/usr/local/opt/libomp/include") + link_directories("/usr/local/opt/libomp/lib") +else() + find_package(OpenMP) + if (OPENMP_FOUND) SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${OpenMP_CXX_FLAGS}") SET(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${OpenMP_EXE_LINKER_FLAGS}") endif() From 88c986fa4f41a3dc277f70e3502f031ea471fde2 Mon Sep 17 00:00:00 2001 From: Scott Inglis Date: Wed, 6 Mar 2019 13:07:07 -0800 Subject: [PATCH 04/14] - Forcing libomp to create a link --- build/ci/phase-template.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/ci/phase-template.yml b/build/ci/phase-template.yml index e0818c796f..987b04cffe 100644 --- a/build/ci/phase-template.yml +++ b/build/ci/phase-template.yml @@ -38,7 +38,7 @@ phases: - script: $(_buildScript) -- /t:DownloadExternalTestFiles /p:IncludeBenchmarkData=$(_includeBenchmarkData) displayName: Download Benchmark Data - ${{ if eq(parameters.name, 'MacOS') }}: - - script: brew update && brew install libomp mono-libgdiplus gettext && brew link gettext --force + - script: brew update && brew install libomp mono-libgdiplus gettext && brew link gettext --force && brew link libomp --force displayName: Install runtime dependencies - script: $(_buildScript) -$(_configuration) -runtests -coverage=$(_codeCoverage) displayName: Run Tests. From 6413c7a6b044ce5cadfe99d087a20a8fa62f72c6 Mon Sep 17 00:00:00 2001 From: Scott Inglis Date: Wed, 6 Mar 2019 13:28:31 -0800 Subject: [PATCH 05/14] Apple test for building... --- src/Native/SymSgdNative/CMakeLists.txt | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Native/SymSgdNative/CMakeLists.txt b/src/Native/SymSgdNative/CMakeLists.txt index 35c80bc5a6..12ca804f69 100644 --- a/src/Native/SymSgdNative/CMakeLists.txt +++ b/src/Native/SymSgdNative/CMakeLists.txt @@ -13,8 +13,10 @@ if(APPLE) # and the else condition can be used instead. SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Xpreprocessor -fopenmp") SET(OPENMP_LIBRARY "omp") - include_directories("/usr/local/opt/libomp/include") - link_directories("/usr/local/opt/libomp/lib") + include_directories("/usr/local/Cellar/libomp/7.0.0/include") + link_directories("/usr/local/Cellar/libomp/7.0.0/lib") + #include_directories("/usr/local/opt/libomp/include") + #link_directories("/usr/local/opt/libomp/lib") else() find_package(OpenMP) if (OPENMP_FOUND) From 8337e779e2d3459c91ab4b50981148dfeb467fa0 Mon Sep 17 00:00:00 2001 From: Scott Inglis Date: Wed, 6 Mar 2019 13:51:31 -0800 Subject: [PATCH 06/14] - Moving the install of libomp to happen before build for Apple --- build/ci/phase-template.yml | 6 +++--- src/Native/SymSgdNative/CMakeLists.txt | 6 ++---- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/build/ci/phase-template.yml b/build/ci/phase-template.yml index 987b04cffe..467ed62baa 100644 --- a/build/ci/phase-template.yml +++ b/build/ci/phase-template.yml @@ -33,13 +33,13 @@ phases: ${{ insert }}: ${{ parameters.customMatrixes }} ${{ insert }}: ${{ parameters.queue }} steps: + - ${{ if eq(parameters.name, 'MacOS') }}: + - script: brew update && brew install libomp mono-libgdiplus gettext && brew link gettext --force && brew link libomp --force + displayName: Install build dependencies - script: $(_buildScript) -$(_configuration) -buildArch=$(_arch) displayName: Build - script: $(_buildScript) -- /t:DownloadExternalTestFiles /p:IncludeBenchmarkData=$(_includeBenchmarkData) displayName: Download Benchmark Data - - ${{ if eq(parameters.name, 'MacOS') }}: - - script: brew update && brew install libomp mono-libgdiplus gettext && brew link gettext --force && brew link libomp --force - displayName: Install runtime dependencies - script: $(_buildScript) -$(_configuration) -runtests -coverage=$(_codeCoverage) displayName: Run Tests. - script: $(Build.SourcesDirectory)/Tools/dotnetcli/dotnet msbuild build/Codecoverage.proj /p:CodeCovToken=$(CODECOV_TOKEN) diff --git a/src/Native/SymSgdNative/CMakeLists.txt b/src/Native/SymSgdNative/CMakeLists.txt index 12ca804f69..35c80bc5a6 100644 --- a/src/Native/SymSgdNative/CMakeLists.txt +++ b/src/Native/SymSgdNative/CMakeLists.txt @@ -13,10 +13,8 @@ if(APPLE) # and the else condition can be used instead. SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Xpreprocessor -fopenmp") SET(OPENMP_LIBRARY "omp") - include_directories("/usr/local/Cellar/libomp/7.0.0/include") - link_directories("/usr/local/Cellar/libomp/7.0.0/lib") - #include_directories("/usr/local/opt/libomp/include") - #link_directories("/usr/local/opt/libomp/lib") + include_directories("/usr/local/opt/libomp/include") + link_directories("/usr/local/opt/libomp/lib") else() find_package(OpenMP) if (OPENMP_FOUND) From b2bde4fc0079c10c02f1a5f0f7805c6d9837cf12 Mon Sep 17 00:00:00 2001 From: Scott Inglis Date: Wed, 6 Mar 2019 14:36:48 -0800 Subject: [PATCH 07/14] - Updating from feedback --- build/ci/phase-template.yml | 5 ++++- build/vsts-ci.yml | 2 ++ src/Native/SymSgdNative/CMakeLists.txt | 1 + src/Native/SymSgdNative/SymSgdNative.cpp | 8 ++++++++ 4 files changed, 15 insertions(+), 1 deletion(-) diff --git a/build/ci/phase-template.yml b/build/ci/phase-template.yml index 467ed62baa..32ca6290e1 100644 --- a/build/ci/phase-template.yml +++ b/build/ci/phase-template.yml @@ -34,10 +34,13 @@ phases: ${{ insert }}: ${{ parameters.queue }} steps: - ${{ if eq(parameters.name, 'MacOS') }}: - - script: brew update && brew install libomp mono-libgdiplus gettext && brew link gettext --force && brew link libomp --force + - script: brew update && brew install libomp && brew link libomp --force displayName: Install build dependencies - script: $(_buildScript) -$(_configuration) -buildArch=$(_arch) displayName: Build + - ${{ if eq(parameters.name, 'MacOS') }}: + - script: brew update && brew install mono-libgdiplus gettext && brew link gettext --force + displayName: Install runtime dependencies - script: $(_buildScript) -- /t:DownloadExternalTestFiles /p:IncludeBenchmarkData=$(_includeBenchmarkData) displayName: Download Benchmark Data - script: $(_buildScript) -$(_configuration) -runtests -coverage=$(_codeCoverage) diff --git a/build/vsts-ci.yml b/build/vsts-ci.yml index b27d86ce7d..4160f383d4 100644 --- a/build/vsts-ci.yml +++ b/build/vsts-ci.yml @@ -49,6 +49,8 @@ phases: - agent.os -equals Darwin steps: # Only build native assets to avoid conflicts. + - script: brew update && brew install libomp && brew link libomp --force + displayName: Install build dependencies - script: ./build.sh -buildNative -$(BuildConfig) -skipRIDAgnosticAssets displayName: Build diff --git a/src/Native/SymSgdNative/CMakeLists.txt b/src/Native/SymSgdNative/CMakeLists.txt index 35c80bc5a6..bf64d91dfa 100644 --- a/src/Native/SymSgdNative/CMakeLists.txt +++ b/src/Native/SymSgdNative/CMakeLists.txt @@ -35,6 +35,7 @@ if(NOT WIN32) endif() endif() +add_definitions(-DUSE_OMP) add_library(SymSgdNative SHARED ${SOURCES} ${RESOURCES}) target_link_libraries(SymSgdNative PUBLIC ${MKL_LIBRARY} PUBLIC ${OPENMP_LIBRARY}) diff --git a/src/Native/SymSgdNative/SymSgdNative.cpp b/src/Native/SymSgdNative/SymSgdNative.cpp index 14eab5ca62..11e85f05e0 100644 --- a/src/Native/SymSgdNative/SymSgdNative.cpp +++ b/src/Native/SymSgdNative/SymSgdNative.cpp @@ -6,7 +6,9 @@ #include #include +#if defined(USE_OMP) #include +#endif #include #include #include "../Stdafx.h" @@ -238,6 +240,7 @@ void InitializeState(int totalNumInstances, int* instSizes, int** instIndices, f TuneNumLocIter(numLocIter, totalNumInstances, instSizes, numThreads); state->WeightScaling = 1.0f; +#if defined(USE_OMP) if (numThreads > 1) { state->PassIteration = 0; @@ -265,6 +268,7 @@ void InitializeState(int totalNumInstances, int* instSizes, int** instIndices, f // To make sure that MKL runs sequentially omp_set_num_threads(1); +#endif } float Loss(int instSize, int* instIndices, float* instValues, @@ -357,6 +361,7 @@ EXPORT_API(void) LearnAll(int totalNumInstances, int* instSizes, int** instIndic } } else { +#if defined(USE_OMP) // In parallel case... bool shouldRemap = !((std::unordered_map*)state->FreqFeatUnorderedMap)->empty(); SymSGD** learners = (SymSGD**)(state->Learners); @@ -443,6 +448,7 @@ EXPORT_API(void) LearnAll(int totalNumInstances, int* instSizes, int** instIndic } } state->TotalInstancesProcessed += numPasses*totalNumInstances; +#endif } } @@ -465,8 +471,10 @@ EXPORT_API(void) MapBackWeightVector(float* weightVector, SymSGDState* state) // Deallocation method EXPORT_API(void) DeallocateSequentially(SymSGDState* state) { +#if defined(USE_OMP) // To make sure that for the rest of MKL calls use parallelism omp_set_num_threads(omp_get_num_procs()); +#endif SymSGD** learners = (SymSGD**)(state->Learners); if (learners) From c1b256575167b9d2300bfde3f91abbf0c3191305 Mon Sep 17 00:00:00 2001 From: Scott Inglis Date: Tue, 12 Mar 2019 18:38:11 -0700 Subject: [PATCH 08/14] - Updating machine images --- .vsts-dotnet-ci.yml | 4 ++-- build/vsts-ci.yml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.vsts-dotnet-ci.yml b/.vsts-dotnet-ci.yml index 1aab0d9cd1..203d89c5ea 100644 --- a/.vsts-dotnet-ci.yml +++ b/.vsts-dotnet-ci.yml @@ -5,10 +5,10 @@ resources: containers: - container: CentosContainer - image: microsoft/dotnet-buildtools-prereqs:centos-7-b46d863-20180719033416 + image: microsoft/dotnet-buildtools-prereqs:centos-7-mlnet-df8e152-20190312232803 - container: UbuntuContainer - image: microsoft/dotnet-buildtools-prereqs:ubuntu-16.04-10fcdcf-20190208200917 + image: microsoft/dotnet-buildtools-prereqs:ubuntu-16.04-mlnet-207e097-20190312152303 phases: - template: /build/ci/phase-template.yml diff --git a/build/vsts-ci.yml b/build/vsts-ci.yml index be7baf979c..3a616b2d15 100644 --- a/build/vsts-ci.yml +++ b/build/vsts-ci.yml @@ -5,7 +5,7 @@ resources: containers: - container: CentosContainer - image: microsoft/dotnet-buildtools-prereqs:centos-7-b46d863-20180719033416 + image: microsoft/dotnet-buildtools-prereqs:centos-7-mlnet-df8e152-20190312232803 phases: ################################################################################ From dd997350110c3b4471b31970c11374de369d7f31 Mon Sep 17 00:00:00 2001 From: Scott Inglis Date: Wed, 13 Mar 2019 07:09:38 -0700 Subject: [PATCH 09/14] - Updating docker images --- .vsts-dotnet-ci.yml | 4 ++-- build/vsts-ci.yml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.vsts-dotnet-ci.yml b/.vsts-dotnet-ci.yml index 203d89c5ea..0c17320a11 100644 --- a/.vsts-dotnet-ci.yml +++ b/.vsts-dotnet-ci.yml @@ -5,10 +5,10 @@ resources: containers: - container: CentosContainer - image: microsoft/dotnet-buildtools-prereqs:centos-7-mlnet-df8e152-20190312232803 + image: mcr.microsoft.com/dotnet-buildtools/prereqs:centos-7-mlnet-df8e152-20190312232803 - container: UbuntuContainer - image: microsoft/dotnet-buildtools-prereqs:ubuntu-16.04-mlnet-207e097-20190312152303 + image: mcr.microsoft.com/dotnet-buildtools/prereqs:ubuntu-16.04-mlnet-207e097-20190312152303 phases: - template: /build/ci/phase-template.yml diff --git a/build/vsts-ci.yml b/build/vsts-ci.yml index 3a616b2d15..a2792204c3 100644 --- a/build/vsts-ci.yml +++ b/build/vsts-ci.yml @@ -5,7 +5,7 @@ resources: containers: - container: CentosContainer - image: microsoft/dotnet-buildtools-prereqs:centos-7-mlnet-df8e152-20190312232803 + image: mcr.microsoft.com/dotnet-buildtools/prereqs:centos-7-mlnet-df8e152-20190312232803 phases: ################################################################################ From 6cdad9d3c3c3efc973716019b9b54d1e1e7a7ad5 Mon Sep 17 00:00:00 2001 From: Scott Inglis Date: Thu, 14 Mar 2019 13:20:24 -0700 Subject: [PATCH 10/14] - Updating centos container, addressing feedback --- .vsts-dotnet-ci.yml | 2 +- build/ci/phase-template.yml | 3 +-- build/vsts-ci.yml | 4 ++-- src/Native/SymSgdNative/SymSgdNative.cpp | 2 +- 4 files changed, 5 insertions(+), 6 deletions(-) diff --git a/.vsts-dotnet-ci.yml b/.vsts-dotnet-ci.yml index 0c17320a11..ff6f43f314 100644 --- a/.vsts-dotnet-ci.yml +++ b/.vsts-dotnet-ci.yml @@ -5,7 +5,7 @@ resources: containers: - container: CentosContainer - image: mcr.microsoft.com/dotnet-buildtools/prereqs:centos-7-mlnet-df8e152-20190312232803 + image: mcr.microsoft.com/dotnet-buildtools/prereqs:centos-7-mlnet-8bba86b-20190314145033 - container: UbuntuContainer image: mcr.microsoft.com/dotnet-buildtools/prereqs:ubuntu-16.04-mlnet-207e097-20190312152303 diff --git a/build/ci/phase-template.yml b/build/ci/phase-template.yml index 32ca6290e1..c80c2ed567 100644 --- a/build/ci/phase-template.yml +++ b/build/ci/phase-template.yml @@ -34,12 +34,11 @@ phases: ${{ insert }}: ${{ parameters.queue }} steps: - ${{ if eq(parameters.name, 'MacOS') }}: - - script: brew update && brew install libomp && brew link libomp --force + - script: brew update && brew install libomp && brew install mono-libgdiplus gettext && brew link gettext --force && brew link libomp --force displayName: Install build dependencies - script: $(_buildScript) -$(_configuration) -buildArch=$(_arch) displayName: Build - ${{ if eq(parameters.name, 'MacOS') }}: - - script: brew update && brew install mono-libgdiplus gettext && brew link gettext --force displayName: Install runtime dependencies - script: $(_buildScript) -- /t:DownloadExternalTestFiles /p:IncludeBenchmarkData=$(_includeBenchmarkData) displayName: Download Benchmark Data diff --git a/build/vsts-ci.yml b/build/vsts-ci.yml index a2792204c3..f2af7ddc1f 100644 --- a/build/vsts-ci.yml +++ b/build/vsts-ci.yml @@ -5,7 +5,7 @@ resources: containers: - container: CentosContainer - image: mcr.microsoft.com/dotnet-buildtools/prereqs:centos-7-mlnet-df8e152-20190312232803 + image: mcr.microsoft.com/dotnet-buildtools/prereqs:centos-7-mlnet-8bba86b-20190314145033 phases: ################################################################################ @@ -48,9 +48,9 @@ phases: demands: - agent.os -equals Darwin steps: - # Only build native assets to avoid conflicts. - script: brew update && brew install libomp && brew link libomp --force displayName: Install build dependencies + # Only build native assets to avoid conflicts. - script: ./build.sh -buildNative -$(BuildConfig) -skipRIDAgnosticAssets displayName: Build diff --git a/src/Native/SymSgdNative/SymSgdNative.cpp b/src/Native/SymSgdNative/SymSgdNative.cpp index 11e85f05e0..680bc80798 100644 --- a/src/Native/SymSgdNative/SymSgdNative.cpp +++ b/src/Native/SymSgdNative/SymSgdNative.cpp @@ -486,4 +486,4 @@ EXPORT_API(void) DeallocateSequentially(SymSGDState* state) delete (std::unordered_map*)state->FreqFeatUnorderedMap; if (state->FreqFeatDirectMap) delete[] state->FreqFeatDirectMap; -} +} \ No newline at end of file From 3a9e9904ddc7fb7bb108ebac0c56c50398efd220 Mon Sep 17 00:00:00 2001 From: Scott Inglis Date: Thu, 14 Mar 2019 14:25:29 -0700 Subject: [PATCH 11/14] -fixing yml file. --- build/ci/phase-template.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/build/ci/phase-template.yml b/build/ci/phase-template.yml index c80c2ed567..cdb4928dbb 100644 --- a/build/ci/phase-template.yml +++ b/build/ci/phase-template.yml @@ -38,8 +38,6 @@ phases: displayName: Install build dependencies - script: $(_buildScript) -$(_configuration) -buildArch=$(_arch) displayName: Build - - ${{ if eq(parameters.name, 'MacOS') }}: - displayName: Install runtime dependencies - script: $(_buildScript) -- /t:DownloadExternalTestFiles /p:IncludeBenchmarkData=$(_includeBenchmarkData) displayName: Download Benchmark Data - script: $(_buildScript) -$(_configuration) -runtests -coverage=$(_codeCoverage) From 6bcf59266fac2076d9ff6a902cd873421b551f0e Mon Sep 17 00:00:00 2001 From: Scott Inglis Date: Fri, 15 Mar 2019 14:18:29 -0700 Subject: [PATCH 12/14] -Minor cmakefile cleanup --- src/Native/SymSgdNative/CMakeLists.txt | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Native/SymSgdNative/CMakeLists.txt b/src/Native/SymSgdNative/CMakeLists.txt index bf64d91dfa..80665883b9 100644 --- a/src/Native/SymSgdNative/CMakeLists.txt +++ b/src/Native/SymSgdNative/CMakeLists.txt @@ -15,19 +15,17 @@ if(APPLE) SET(OPENMP_LIBRARY "omp") include_directories("/usr/local/opt/libomp/include") link_directories("/usr/local/opt/libomp/lib") + + list(APPEND SOURCES ${VERSION_FILE_PATH}) else() find_package(OpenMP) if (OPENMP_FOUND) SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${OpenMP_CXX_FLAGS}") SET(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${OpenMP_EXE_LINKER_FLAGS}") endif() -endif() - -find_library(MKL_LIBRARY MklImports HINTS ${MKL_LIB_PATH}) -if(NOT WIN32) - list(APPEND SOURCES ${VERSION_FILE_PATH}) - if(NOT APPLE) + if (NOT WIN32) + list(APPEND SOURCES ${VERSION_FILE_PATH}) SET(CMAKE_SKIP_BUILD_RPATH FALSE) SET(CMAKE_BUILD_WITH_INSTALL_RPATH FALSE) SET(CMAKE_INSTALL_RPATH_USE_LINK_PATH TRUE) @@ -35,6 +33,8 @@ if(NOT WIN32) endif() endif() +find_library(MKL_LIBRARY MklImports HINTS ${MKL_LIB_PATH}) + add_definitions(-DUSE_OMP) add_library(SymSgdNative SHARED ${SOURCES} ${RESOURCES}) target_link_libraries(SymSgdNative PUBLIC ${MKL_LIBRARY} PUBLIC ${OPENMP_LIBRARY}) From f4e528d0dca0ee1532b24b0acc49ce77164277fc Mon Sep 17 00:00:00 2001 From: Scott Inglis Date: Fri, 15 Mar 2019 16:49:30 -0700 Subject: [PATCH 13/14] - NumberOfThreads defaults to Environment.ProcessorCount, comment updates --- .../SymSgdClassificationTrainer.cs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/Microsoft.ML.Mkl.Components/SymSgdClassificationTrainer.cs b/src/Microsoft.ML.Mkl.Components/SymSgdClassificationTrainer.cs index 3246ea9762..261fb85f3e 100644 --- a/src/Microsoft.ML.Mkl.Components/SymSgdClassificationTrainer.cs +++ b/src/Microsoft.ML.Mkl.Components/SymSgdClassificationTrainer.cs @@ -44,7 +44,7 @@ public sealed class Options : TrainerInputBaseWithLabel { /// /// Degree of lock-free parallelism. Determinism not guaranteed if this is set to higher than 1. - /// Multi-threading is not supported currently. + /// The default value is the number of logical cores that are available on the system. /// [Argument(ArgumentType.AtMostOnce, HelpText = "Degree of lock-free parallelism. Determinism not guaranteed. " + "Multi-threading is not supported currently.", ShortName = "nt")] @@ -663,10 +663,7 @@ private TPredictor TrainCore(IChannel ch, RoleMappedData data, LinearModelParame { int numFeatures = data.Schema.Feature.Value.Type.GetVectorSize(); var cursorFactory = new FloatLabelCursor.Factory(data, CursOpt.Label | CursOpt.Features); - int numThreads = 1; - - if (_options.NumberOfThreads.HasValue) - numThreads = _options.NumberOfThreads.Value; + int numThreads = _options.NumberOfThreads ?? Environment.ProcessorCount; ch.CheckUserArg(numThreads > 0, nameof(_options.NumberOfThreads), "The number of threads must be either null or a positive integer."); From 1b7f9514f6c7a560872125569cd2c6f2f74e7e8b Mon Sep 17 00:00:00 2001 From: Scott Inglis Date: Fri, 15 Mar 2019 17:55:06 -0700 Subject: [PATCH 14/14] - Setting SymSSGD test to use 1 thread for consistency --- test/Microsoft.ML.Tests/FeatureContributionTests.cs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/test/Microsoft.ML.Tests/FeatureContributionTests.cs b/test/Microsoft.ML.Tests/FeatureContributionTests.cs index ba3f83e5d6..599e03437c 100644 --- a/test/Microsoft.ML.Tests/FeatureContributionTests.cs +++ b/test/Microsoft.ML.Tests/FeatureContributionTests.cs @@ -158,14 +158,22 @@ public void TestSDCABinary() public void TestSGDBinary() { TestFeatureContribution(ML.BinaryClassification.Trainers.SgdCalibrated( - new SgdCalibratedTrainer.Options { NumberOfThreads = 1 }), + new SgdCalibratedTrainer.Options() + { + NumberOfThreads = 1 + }), GetSparseDataset(TaskType.BinaryClassification, 100), "SGDBinary"); } [Fact] public void TestSSGDBinary() { - TestFeatureContribution(ML.BinaryClassification.Trainers.SymbolicSgd(), GetSparseDataset(TaskType.BinaryClassification, 100), "SSGDBinary", 4); + TestFeatureContribution(ML.BinaryClassification.Trainers.SymbolicSgd( + new SymbolicSgdTrainer.Options() + { + NumberOfThreads = 1 + }), + GetSparseDataset(TaskType.BinaryClassification, 100), "SSGDBinary", 4); } [Fact]