From 78f813bee27c337c471892122266219bf48cc4d1 Mon Sep 17 00:00:00 2001 From: Viktor Gal Date: Tue, 21 Feb 2017 20:15:19 +0800 Subject: [PATCH 01/24] Make c++11 features a hard requirement for shogun --- CMakeLists.txt | 63 ++----- NEWS | 3 +- cmake/CheckCXX11Features.cmake | 164 ------------------ .../cxx11-test-__func__.cpp | 8 - .../CheckCXX11Features/cxx11-test-atomic.cpp | 13 -- cmake/CheckCXX11Features/cxx11-test-auto.cpp | 12 -- .../cxx11-test-auto_fail_compile.cpp | 7 - .../cxx11-test-auto_ret_type.cpp | 8 - .../cxx11-test-class_override_final.cpp | 21 --- ...test-class_override_final_fail_compile.cpp | 25 --- .../cxx11-test-constexpr.cpp | 19 -- .../CheckCXX11Features/cxx11-test-cstdint.cpp | 11 -- .../cxx11-test-decltype.cpp | 10 -- .../cxx11-test-initializer_list.cpp | 27 --- .../CheckCXX11Features/cxx11-test-lambda.cpp | 5 - .../cxx11-test-long_long.cpp | 7 - .../CheckCXX11Features/cxx11-test-nullptr.cpp | 6 - .../cxx11-test-nullptr_fail_compile.cpp | 6 - cmake/CheckCXX11Features/cxx11-test-regex.cpp | 26 --- .../cxx11-test-rvalue-references.cpp | 57 ------ .../cxx11-test-sizeof_member.cpp | 14 -- .../cxx11-test-sizeof_member_fail.cpp | 9 - .../cxx11-test-static_assert.cpp | 5 - .../cxx11-test-static_assert_fail_compile.cpp | 5 - .../cxx11-test-variadic_templates.cpp | 23 --- src/interfaces/python_modular/CMakeLists.txt | 11 -- 26 files changed, 21 insertions(+), 544 deletions(-) delete mode 100644 cmake/CheckCXX11Features.cmake delete mode 100755 cmake/CheckCXX11Features/cxx11-test-__func__.cpp delete mode 100755 cmake/CheckCXX11Features/cxx11-test-atomic.cpp delete mode 100755 cmake/CheckCXX11Features/cxx11-test-auto.cpp delete mode 100755 cmake/CheckCXX11Features/cxx11-test-auto_fail_compile.cpp delete mode 100755 cmake/CheckCXX11Features/cxx11-test-auto_ret_type.cpp delete mode 100755 cmake/CheckCXX11Features/cxx11-test-class_override_final.cpp delete mode 100755 cmake/CheckCXX11Features/cxx11-test-class_override_final_fail_compile.cpp delete mode 100755 cmake/CheckCXX11Features/cxx11-test-constexpr.cpp delete mode 100755 cmake/CheckCXX11Features/cxx11-test-cstdint.cpp delete mode 100755 cmake/CheckCXX11Features/cxx11-test-decltype.cpp delete mode 100755 cmake/CheckCXX11Features/cxx11-test-initializer_list.cpp delete mode 100755 cmake/CheckCXX11Features/cxx11-test-lambda.cpp delete mode 100755 cmake/CheckCXX11Features/cxx11-test-long_long.cpp delete mode 100755 cmake/CheckCXX11Features/cxx11-test-nullptr.cpp delete mode 100755 cmake/CheckCXX11Features/cxx11-test-nullptr_fail_compile.cpp delete mode 100755 cmake/CheckCXX11Features/cxx11-test-regex.cpp delete mode 100755 cmake/CheckCXX11Features/cxx11-test-rvalue-references.cpp delete mode 100755 cmake/CheckCXX11Features/cxx11-test-sizeof_member.cpp delete mode 100755 cmake/CheckCXX11Features/cxx11-test-sizeof_member_fail.cpp delete mode 100755 cmake/CheckCXX11Features/cxx11-test-static_assert.cpp delete mode 100755 cmake/CheckCXX11Features/cxx11-test-static_assert_fail_compile.cpp delete mode 100755 cmake/CheckCXX11Features/cxx11-test-variadic_templates.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 0da0ebc4532..9f6901104c9 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -31,6 +31,17 @@ include(ShogunUtils) SET(CMAKE_INCLUDE_DIRECTORIES_PROJECT_BEFORE ON) +#### set required C++ standard level of the compiler +set(CMAKE_CXX_STANDARD 11) +set(CMAKE_CXX_STANDARD_REQUIRED ON) +set(CMAKE_CXX_EXTENSIONS OFF) + +### FIXME: remove these flags when the codebase +# is cleared up +set(HAVE_CXX11 ON) +set(HAVE_CXX11_ATOMIC ON) +set(HAVE_STD_UNORDERED_MAP ON) + ############# minimum library versions ################### SET(EIGEN_VERSION_MINIMUM 3.1.2) SET(VIENNACL_VERSION_MINIMUM 1.5.0) @@ -90,13 +101,9 @@ SET(LIBSHOGUN ON CACHE BOOL "Compile shogun library") IsAnyTrue("${AVAILABLE_INTERFACES}" ANY_INTERFACE_ENABLED) IF (${ANY_INTERFACE_ENABLED}) - IF(RModular) - FIND_PACKAGE(SWIG 2.0.5 REQUIRED) - ELSE() - FIND_PACKAGE(SWIG 2.0.4 REQUIRED) - ENDIF() + # SWIG3 is the minimum requirement because of C++11 support + SET(SWIG_VERSION_MINIMUM 3.0.0) - SET(SWIG_VERSION_MINIMUM 2.0.4) IF(CSharpModular) # We require SWIG 3.0.7 to support functions with a few SGVector or # SGMatrix arguments. The required SWIG feature is called @@ -108,11 +115,14 @@ IF (${ANY_INTERFACE_ENABLED}) # typemapping created for earlier versions of SWIG. # see: http://www.swig.org/Doc3.0/CSharp.html#CSharp_introduction_swig2_compatibility LIST(APPEND CMAKE_SWIG_FLAGS "-DSWIG2_CSHARP") - ELSEIF(RModular) - SET(SWIG_VERSION_MINIMUM 2.0.5) + ELSEIF(PythonModular) + # SWIG was broken for combining -builtin and -modernargs + # from v3.0.0 and until 3.0.4. This bug was fixed in + # v3.0.5. Make CMake emit an error and fail to configure. + SET(SWIG_VERSION_MINIMUM 3.0.5) ENDIF() - FIND_PACKAGE(SWIG ${SWIG_VERSION_MINIMUM} REQUIRED) + FIND_PACKAGE(SWIG ${SWIG_VERSION_MINIMUM} REQUIRED) # use our own UseSWIG.cmake in order to be able to enable ccache-swig SET(SWIG_USE_FILE ${CMAKE_SOURCE_DIR}/cmake/UseSWIG.cmake) IF(ENABLE_CCACHE AND CCACHE_SWIG) @@ -247,41 +257,6 @@ IF(ENABLE_UBSAN) ENDIF() ENDIF() -# check for supported c++11 features -# -# clang with -std=c++11 and -stdlib=libc++ does not work -# well with swig generated cxx hence disable c++11 for this case. - -# this has been only fixed in swig 2.0.12 or later. -IF (NOT ((CYGWIN AND ENABLE_TESTING) OR (DARWIN AND COMPILE_MODULAR_INTERFACE - AND SWIG_VERSION VERSION_LESS "2.0.12"))) - INCLUDE(CheckCXX11Features) - - IF (MSVC) - IF(MSVC_VERSION VERSION_GREATER "1500") - SET(HAVE_CXX11 1) - ENDIF() - ELSE(MSVC) - IF(_HAS_CXX11_FLAG) - SET(HAVE_CXX11 1) - SET(CMAKE_CXX_FLAGS "${CXX11_COMPILER_FLAGS} ${CMAKE_CXX_FLAGS}") - SET(SWIG_CXX_COMPILER_FLAGS "${CXX11_COMPILER_FLAGS} ${SWIG_CXX_COMPILER_FLAGS}") - ENDIF() - - IF(_HAS_CXX0X_FLAG) - SET(HAVE_CXX0X 1) - SET(CMAKE_CXX_FLAGS "${CXX11_COMPILER_FLAGS} ${CMAKE_CXX_FLAGS}") - SET(SWIG_CXX_COMPILER_FLAGS "${CXX11_COMPILER_FLAGS} ${SWIG_CXX_COMPILER_FLAGS}") - ENDIF() - ENDIF() -ELSEIF(DARWIN AND ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang") AND NOT (CMAKE_CXX_COMPILER_VERSION VERSION_LESS "5.0.0")) - # osx clang 5.0.0 or later uses libc++ by default - # this is causing problems with source generated by swig version earlier than 3.0.0 - # force to use libstdc++ for compilation of sources - SET(CMAKE_CXX_FLAGS "-stdlib=libstdc++ ${CMAKE_CXX_FLAGS}") - SET(SWIG_CXX_COMPILER_FLAGS "-stdlib=libstdc++ ${SWIG_CXX_COMPILER_FLAGS}") -ENDIF() - # Fix build on Mac OSX 10.10 Yosemite when using mp-gcc-4X. # See: https://github.com/shogun-toolbox/shogun/issues/2635 IF(DARWIN AND (NOT "${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang")) diff --git a/NEWS b/NEWS index 2329ebd061d..041495311b2 100644 --- a/NEWS +++ b/NEWS @@ -1,9 +1,10 @@ 2016-11-05 Viktor Gal - * SHOGUN Release version 5.1.0 (libshogun 17.2, data 0.11, parameter 1) + * SHOGUN Release version 6.0.0 (libshogun 18.0, data 0.11, parameter 1) * NOTE: Contains major rewrite and clean-up of developer documentation in doc/readme [Heiko Strathmann] * Add native MS Windows support [Viktor Gal] + * Shogun requires the compiler to support C++11 features * Features: - LDA now supports 32, 64 and 128 bit floating point numbers [Chris Goldsworthy] diff --git a/cmake/CheckCXX11Features.cmake b/cmake/CheckCXX11Features.cmake deleted file mode 100644 index ee2b8b478c8..00000000000 --- a/cmake/CheckCXX11Features.cmake +++ /dev/null @@ -1,164 +0,0 @@ -# - Check which parts of the C++11 standard the compiler supports -# -# When found it will set the following variables -# -# CXX11_COMPILER_FLAGS - the compiler flags needed to get C++11 features -# -# HAVE_CXX11_ATOMIC - atomic template -# HAVE_CXX11_AUTO - auto keyword -# HAVE_CXX11_AUTO_RET_TYPE - function declaration with deduced return types -# HAVE_CXX11_CLASS_OVERRIDE - override and final keywords for classes and methods -# HAVE_CXX11_CONSTEXPR - constexpr keyword -# HAVE_CXX11_CSTDINT_H - cstdint header -# HAVE_CXX11_DECLTYPE - decltype keyword -# HAVE_CXX11_FUNC - __func__ preprocessor constant -# HAVE_CXX11_INITIALIZER_LIST - initializer list -# HAVE_CXX11_LAMBDA - lambdas -# HAVE_CXX11_LIB_REGEX - regex library -# HAVE_CXX11_LONG_LONG - long long signed & unsigned types -# HAVE_CXX11_NULLPTR - nullptr -# HAVE_CXX11_RVALUE_REFERENCES - rvalue references -# HAVE_CXX11_SIZEOF_MEMBER - sizeof() non-static members -# HAVE_CXX11_STATIC_ASSERT - static_assert() -# HAVE_CXX11_VARIADIC_TEMPLATES - variadic templates - -#============================================================================= -# Copyright 2011,2012 Rolf Eike Beer -# Copyright 2012 Andreas Weis -# Copyright 2013 Viktor -# -# Distributed under the OSI-approved BSD License (the "License"); -# see accompanying file Copyright.txt for details. -# -# This software is distributed WITHOUT ANY WARRANTY; without even the -# implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. -# See the License for more information. -#============================================================================= -# (To distribute this file outside of CMake, substitute the full -# License text for the above reference.) - -# -# Each feature may have up to 3 checks, every one of them in it's own file -# FEATURE.cpp - example that must build and return 0 when run -# FEATURE_fail.cpp - example that must build, but may not return 0 when run -# FEATURE_fail_compile.cpp - example that must fail compilation -# -# The first one is mandatory, the latter 2 are optional and do not depend on -# each other (i.e. only one may be present). -# - -if (NOT CMAKE_CXX_COMPILER_LOADED) - message(FATAL_ERROR "CheckCXX11Features modules only works if language CXX is enabled") -endif () - -cmake_minimum_required(VERSION 2.8.3) - -# -### Check for needed compiler flags -# -include(CheckCXXCompilerFlag) -check_cxx_compiler_flag("-std=c++11" _HAS_CXX11_FLAG) -if (NOT _HAS_CXX11_FLAG) - check_cxx_compiler_flag("-std=c++0x" _HAS_CXX0X_FLAG) -endif () - -if (_HAS_CXX11_FLAG) - # apple's clang requires -stdlib=libc++ otherwise - # it won't find for example. - # but this will break compilation on ubuntu - # - # TODO: investigate further which system requires -stdlib=libc++ - # as well. - if ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang" AND DARWIN) - set(CXX11_COMPILER_FLAGS "-std=c++11 -stdlib=libc++") - else () - set(CXX11_COMPILER_FLAGS "-std=c++11") - endif () -elseif (_HAS_CXX0X_FLAG) - # to avoid problems with variadic template handling in gcc 4.6.3 - # with -std=c++0x mode in gmock-matchers.h - # error: unimplemented: cannot expand ‘Tail ...’ into a fixed-length argument list - # TODO: check if there's a version - IF("${CMAKE_CXX_COMPILER_VERSION}" VERSION_GREATER "4.6.3") - set(CXX11_COMPILER_FLAGS "-std=c++0x") - ELSE() - MESSAGE(STATUS "Your compiler cannot expand a variadic template parameter into a fixed-length argument list.") - SET(_HAS_CXX0X_FLAG FALSE) - ENDIF() -endif () - -function(cxx11_check_feature FEATURE_NAME RESULT_VAR) - if (NOT DEFINED ${RESULT_VAR}) - set(_bindir "${CMAKE_CURRENT_BINARY_DIR}/CMakeFiles/CMakeTmp/cxx11_${FEATURE_NAME}") - - set(_SRCFILE_BASE ${CMAKE_CURRENT_LIST_DIR}/CheckCXX11Features/cxx11-test-${FEATURE_NAME}) - set(_LOG_NAME "\"${FEATURE_NAME}\"") - message(STATUS "Checking C++11 support for ${_LOG_NAME}") - - set(_SRCFILE "${_SRCFILE_BASE}.cpp") - set(_SRCFILE_FAIL "${_SRCFILE_BASE}_fail.cpp") - set(_SRCFILE_FAIL_COMPILE "${_SRCFILE_BASE}_fail_compile.cpp") - - if (CROSS_COMPILING) - try_compile(${RESULT_VAR} "${_bindir}" "${_SRCFILE}" - COMPILE_DEFINITIONS "${CXX11_COMPILER_FLAGS}") - if (${RESULT_VAR} AND EXISTS ${_SRCFILE_FAIL}) - try_compile(${RESULT_VAR} "${_bindir}_fail" "${_SRCFILE_FAIL}" - COMPILE_DEFINITIONS "${CXX11_COMPILER_FLAGS}") - endif (${RESULT_VAR} AND EXISTS ${_SRCFILE_FAIL}) - else (CROSS_COMPILING) - try_run(_RUN_RESULT_VAR _COMPILE_RESULT_VAR - "${_bindir}" "${_SRCFILE}" - COMPILE_DEFINITIONS "${CXX11_COMPILER_FLAGS}") - if (_COMPILE_RESULT_VAR AND NOT _RUN_RESULT_VAR) - set(${RESULT_VAR} TRUE) - else (_COMPILE_RESULT_VAR AND NOT _RUN_RESULT_VAR) - set(${RESULT_VAR} FALSE) - endif (_COMPILE_RESULT_VAR AND NOT _RUN_RESULT_VAR) - if (${RESULT_VAR} AND EXISTS ${_SRCFILE_FAIL}) - try_run(_RUN_RESULT_VAR _COMPILE_RESULT_VAR - "${_bindir}_fail" "${_SRCFILE_FAIL}" - COMPILE_DEFINITIONS "${CXX11_COMPILER_FLAGS}") - if (_COMPILE_RESULT_VAR AND _RUN_RESULT_VAR) - set(${RESULT_VAR} TRUE) - else (_COMPILE_RESULT_VAR AND _RUN_RESULT_VAR) - set(${RESULT_VAR} FALSE) - endif (_COMPILE_RESULT_VAR AND _RUN_RESULT_VAR) - endif (${RESULT_VAR} AND EXISTS ${_SRCFILE_FAIL}) - endif (CROSS_COMPILING) - if (${RESULT_VAR} AND EXISTS ${_SRCFILE_FAIL_COMPILE}) - try_compile(_TMP_RESULT "${_bindir}_fail_compile" "${_SRCFILE_FAIL_COMPILE}" - COMPILE_DEFINITIONS "${CXX11_COMPILER_FLAGS}") - if (_TMP_RESULT) - set(${RESULT_VAR} FALSE) - else (_TMP_RESULT) - set(${RESULT_VAR} TRUE) - endif (_TMP_RESULT) - endif (${RESULT_VAR} AND EXISTS ${_SRCFILE_FAIL_COMPILE}) - - if (${RESULT_VAR}) - message(STATUS "Checking C++11 support for ${_LOG_NAME}: works") - else (${RESULT_VAR}) - message(STATUS "Checking C++11 support for ${_LOG_NAME}: not supported") - endif (${RESULT_VAR}) - set(${RESULT_VAR} ${${RESULT_VAR}} CACHE INTERNAL "C++11 support for ${_LOG_NAME}") - endif (NOT DEFINED ${RESULT_VAR}) -endfunction(cxx11_check_feature) - -cxx11_check_feature("__func__" HAVE_CXX11_FUNC) -cxx11_check_feature("atomic" HAVE_CXX11_ATOMIC) -cxx11_check_feature("auto" HAVE_CXX11_AUTO) -cxx11_check_feature("auto_ret_type" HAVE_CXX11_AUTO_RET_TYPE) -cxx11_check_feature("class_override_final" HAVE_CXX11_CLASS_OVERRIDE) -cxx11_check_feature("constexpr" HAVE_CXX11_CONSTEXPR) -cxx11_check_feature("cstdint" HAVE_CXX11_CSTDINT_H) -cxx11_check_feature("decltype" HAVE_CXX11_DECLTYPE) -cxx11_check_feature("initializer_list" HAVE_CXX11_INITIALIZER_LIST) -cxx11_check_feature("lambda" HAVE_CXX11_LAMBDA) -cxx11_check_feature("long_long" HAVE_CXX11_LONG_LONG) -cxx11_check_feature("nullptr" HAVE_CXX11_NULLPTR) -cxx11_check_feature("regex" HAVE_CXX11_LIB_REGEX) -cxx11_check_feature("rvalue-references" HAVE_CXX11_RVALUE_REFERENCES) -cxx11_check_feature("sizeof_member" HAVE_CXX11_SIZEOF_MEMBER) -cxx11_check_feature("static_assert" HAVE_CXX11_STATIC_ASSERT) -cxx11_check_feature("variadic_templates" HAVE_CXX11_VARIADIC_TEMPLATES) diff --git a/cmake/CheckCXX11Features/cxx11-test-__func__.cpp b/cmake/CheckCXX11Features/cxx11-test-__func__.cpp deleted file mode 100755 index 3bfd8a89356..00000000000 --- a/cmake/CheckCXX11Features/cxx11-test-__func__.cpp +++ /dev/null @@ -1,8 +0,0 @@ -int main(void) -{ - if (!__func__) - return 1; - if (!(*__func__)) - return 1; - return 0; -} diff --git a/cmake/CheckCXX11Features/cxx11-test-atomic.cpp b/cmake/CheckCXX11Features/cxx11-test-atomic.cpp deleted file mode 100755 index de14da5ccf9..00000000000 --- a/cmake/CheckCXX11Features/cxx11-test-atomic.cpp +++ /dev/null @@ -1,13 +0,0 @@ -#include - -int main() -{ - volatile std::atomic x; - x.store(0); - - bool ret = ( - x.load() == 0 - ); - - return ret ? 0 : 1; -} \ No newline at end of file diff --git a/cmake/CheckCXX11Features/cxx11-test-auto.cpp b/cmake/CheckCXX11Features/cxx11-test-auto.cpp deleted file mode 100755 index 948648e9936..00000000000 --- a/cmake/CheckCXX11Features/cxx11-test-auto.cpp +++ /dev/null @@ -1,12 +0,0 @@ - -int main() -{ - auto i = 5; - auto f = 3.14159f; - auto d = 3.14159; - bool ret = ( - (sizeof(f) < sizeof(d)) && - (sizeof(i) == sizeof(int)) - ); - return ret ? 0 : 1; -} diff --git a/cmake/CheckCXX11Features/cxx11-test-auto_fail_compile.cpp b/cmake/CheckCXX11Features/cxx11-test-auto_fail_compile.cpp deleted file mode 100755 index 3c0e3f2e015..00000000000 --- a/cmake/CheckCXX11Features/cxx11-test-auto_fail_compile.cpp +++ /dev/null @@ -1,7 +0,0 @@ -int main(void) -{ - // must fail because there is no initializer - auto i; - - return 0; -} diff --git a/cmake/CheckCXX11Features/cxx11-test-auto_ret_type.cpp b/cmake/CheckCXX11Features/cxx11-test-auto_ret_type.cpp deleted file mode 100755 index 937b6835554..00000000000 --- a/cmake/CheckCXX11Features/cxx11-test-auto_ret_type.cpp +++ /dev/null @@ -1,8 +0,0 @@ -auto foo(int i) -> int { - return i - 1; -} - -int main() -{ - return foo(1); -} diff --git a/cmake/CheckCXX11Features/cxx11-test-class_override_final.cpp b/cmake/CheckCXX11Features/cxx11-test-class_override_final.cpp deleted file mode 100755 index f5870b19941..00000000000 --- a/cmake/CheckCXX11Features/cxx11-test-class_override_final.cpp +++ /dev/null @@ -1,21 +0,0 @@ -class base { -public: - virtual int foo(int a) - { return 4 + a; } - int bar(int a) final - { return a - 2; } -}; - -class sub final : public base { -public: - virtual int foo(int a) override - { return 8 + 2 * a; }; -}; - -int main(void) -{ - base b; - sub s; - - return (b.foo(2) * 2 == s.foo(2)) ? 0 : 1; -} diff --git a/cmake/CheckCXX11Features/cxx11-test-class_override_final_fail_compile.cpp b/cmake/CheckCXX11Features/cxx11-test-class_override_final_fail_compile.cpp deleted file mode 100755 index bc00b278ed2..00000000000 --- a/cmake/CheckCXX11Features/cxx11-test-class_override_final_fail_compile.cpp +++ /dev/null @@ -1,25 +0,0 @@ -class base { -public: - virtual int foo(int a) - { return 4 + a; } - virtual int bar(int a) final - { return a - 2; } -}; - -class sub final : public base { -public: - virtual int foo(int a) override - { return 8 + 2 * a; }; - virtual int bar(int a) - { return a; } -}; - -class impossible : public sub { }; - -int main(void) -{ - base b; - sub s; - - return 1; -} diff --git a/cmake/CheckCXX11Features/cxx11-test-constexpr.cpp b/cmake/CheckCXX11Features/cxx11-test-constexpr.cpp deleted file mode 100755 index ed624512d8e..00000000000 --- a/cmake/CheckCXX11Features/cxx11-test-constexpr.cpp +++ /dev/null @@ -1,19 +0,0 @@ -constexpr int square(int x) -{ - return x*x; -} - -constexpr int the_answer() -{ - return 42; -} - -int main() -{ - int test_arr[square(3)]; - bool ret = ( - (square(the_answer()) == 1764) && - (sizeof(test_arr)/sizeof(test_arr[0]) == 9) - ); - return ret ? 0 : 1; -} diff --git a/cmake/CheckCXX11Features/cxx11-test-cstdint.cpp b/cmake/CheckCXX11Features/cxx11-test-cstdint.cpp deleted file mode 100755 index ca2c72ddd09..00000000000 --- a/cmake/CheckCXX11Features/cxx11-test-cstdint.cpp +++ /dev/null @@ -1,11 +0,0 @@ -#include - -int main() -{ - bool test = - (sizeof(int8_t) == 1) && - (sizeof(int16_t) == 2) && - (sizeof(int32_t) == 4) && - (sizeof(int64_t) == 8); - return test ? 0 : 1; -} diff --git a/cmake/CheckCXX11Features/cxx11-test-decltype.cpp b/cmake/CheckCXX11Features/cxx11-test-decltype.cpp deleted file mode 100755 index 0dbb1cc5203..00000000000 --- a/cmake/CheckCXX11Features/cxx11-test-decltype.cpp +++ /dev/null @@ -1,10 +0,0 @@ -bool check_size(int i) -{ - return sizeof(int) == sizeof(decltype(i)); -} - -int main() -{ - bool ret = check_size(42); - return ret ? 0 : 1; -} diff --git a/cmake/CheckCXX11Features/cxx11-test-initializer_list.cpp b/cmake/CheckCXX11Features/cxx11-test-initializer_list.cpp deleted file mode 100755 index 35e6c384290..00000000000 --- a/cmake/CheckCXX11Features/cxx11-test-initializer_list.cpp +++ /dev/null @@ -1,27 +0,0 @@ -#include - -class seq { -public: - seq(std::initializer_list list); - - int length() const; -private: - std::vector m_v; -}; - -seq::seq(std::initializer_list list) - : m_v(list) -{ -} - -int seq::length() const -{ - return m_v.size(); -} - -int main(void) -{ - seq a = {18, 20, 2, 0, 4, 7}; - - return (a.length() == 6) ? 0 : 1; -} diff --git a/cmake/CheckCXX11Features/cxx11-test-lambda.cpp b/cmake/CheckCXX11Features/cxx11-test-lambda.cpp deleted file mode 100755 index 4c33ed58dd7..00000000000 --- a/cmake/CheckCXX11Features/cxx11-test-lambda.cpp +++ /dev/null @@ -1,5 +0,0 @@ -int main() -{ - int ret = 0; - return ([&ret]() -> int { return ret; })(); -} diff --git a/cmake/CheckCXX11Features/cxx11-test-long_long.cpp b/cmake/CheckCXX11Features/cxx11-test-long_long.cpp deleted file mode 100755 index 091112756a7..00000000000 --- a/cmake/CheckCXX11Features/cxx11-test-long_long.cpp +++ /dev/null @@ -1,7 +0,0 @@ -int main(void) -{ - long long l; - unsigned long long ul; - - return ((sizeof(l) >= 8) && (sizeof(ul) >= 8)) ? 0 : 1; -} diff --git a/cmake/CheckCXX11Features/cxx11-test-nullptr.cpp b/cmake/CheckCXX11Features/cxx11-test-nullptr.cpp deleted file mode 100755 index 9f410715314..00000000000 --- a/cmake/CheckCXX11Features/cxx11-test-nullptr.cpp +++ /dev/null @@ -1,6 +0,0 @@ -int main(void) -{ - void *v = nullptr; - - return v ? 1 : 0; -} diff --git a/cmake/CheckCXX11Features/cxx11-test-nullptr_fail_compile.cpp b/cmake/CheckCXX11Features/cxx11-test-nullptr_fail_compile.cpp deleted file mode 100755 index 6a002bcb781..00000000000 --- a/cmake/CheckCXX11Features/cxx11-test-nullptr_fail_compile.cpp +++ /dev/null @@ -1,6 +0,0 @@ -int main(void) -{ - int i = nullptr; - - return 1; -} diff --git a/cmake/CheckCXX11Features/cxx11-test-regex.cpp b/cmake/CheckCXX11Features/cxx11-test-regex.cpp deleted file mode 100755 index 2fe01c4fc96..00000000000 --- a/cmake/CheckCXX11Features/cxx11-test-regex.cpp +++ /dev/null @@ -1,26 +0,0 @@ -#include -#include - -int parse_line(std::string const& line) -{ - std::string tmp; - if(std::regex_search(line, std::regex("(\\s)+(-)?(\\d)+//(-)?(\\d)+(\\s)+"))) { - tmp = std::regex_replace(line, std::regex("(-)?(\\d)+//(-)?(\\d)+"), std::string("V")); - } else if(std::regex_search(line, std::regex("(\\s)+(-)?(\\d)+/(-)?(\\d)+(\\s)+"))) { - tmp = std::regex_replace(line, std::regex("(-)?(\\d)+/(-)?(\\d)+"), std::string("V")); - } else if(std::regex_search(line, std::regex("(\\s)+(-)?(\\d)+/(-)?(\\d)+/(-)?(\\d)+(\\s)+"))) { - tmp = std::regex_replace(line, std::regex("(-)?(\\d)+/(-)?(\\d)+/(-)?(\\d)+"), std::string("V")); - } else { - tmp = std::regex_replace(line, std::regex("(-)?(\\d)+"), std::string("V")); - } - return static_cast(std::count(tmp.begin(), tmp.end(), 'V')); -} - -int main() -{ - bool test = (parse_line("f 7/7/7 -3/3/-3 2/-2/2") == 3) && - (parse_line("f 7//7 3//-3 -2//2") == 3) && - (parse_line("f 7/7 3/-3 -2/2") == 3) && - (parse_line("f 7 3 -2") == 3); - return test ? 0 : 1; -} diff --git a/cmake/CheckCXX11Features/cxx11-test-rvalue-references.cpp b/cmake/CheckCXX11Features/cxx11-test-rvalue-references.cpp deleted file mode 100755 index e6e7e5a98a8..00000000000 --- a/cmake/CheckCXX11Features/cxx11-test-rvalue-references.cpp +++ /dev/null @@ -1,57 +0,0 @@ -#include - -class rvmove { -public: - void *ptr; - char *array; - - rvmove() - : ptr(0), - array(new char[10]) - { - ptr = this; - } - - rvmove(rvmove &&other) - : ptr(other.ptr), - array(other.array) - { - other.array = 0; - other.ptr = 0; - } - - ~rvmove() - { - assert(((ptr != 0) && (array != 0)) || ((ptr == 0) && (array == 0))); - delete[] array; - } - - rvmove &operator=(rvmove &&other) - { - delete[] array; - ptr = other.ptr; - array = other.array; - other.array = 0; - other.ptr = 0; - return *this; - } - - static rvmove create() - { - return rvmove(); - } -private: - rvmove(const rvmove &); - rvmove &operator=(const rvmove &); -}; - -int main() -{ - rvmove mine; - if (mine.ptr != &mine) - return 1; - mine = rvmove::create(); - if (mine.ptr == &mine) - return 1; - return 0; -} diff --git a/cmake/CheckCXX11Features/cxx11-test-sizeof_member.cpp b/cmake/CheckCXX11Features/cxx11-test-sizeof_member.cpp deleted file mode 100755 index 4902fc73eee..00000000000 --- a/cmake/CheckCXX11Features/cxx11-test-sizeof_member.cpp +++ /dev/null @@ -1,14 +0,0 @@ -struct foo { - char bar; - int baz; -}; - -int main(void) -{ - bool ret = ( - (sizeof(foo::bar) == 1) && - (sizeof(foo::baz) >= sizeof(foo::bar)) && - (sizeof(foo) >= sizeof(foo::bar) + sizeof(foo::baz)) - ); - return ret ? 0 : 1; -} diff --git a/cmake/CheckCXX11Features/cxx11-test-sizeof_member_fail.cpp b/cmake/CheckCXX11Features/cxx11-test-sizeof_member_fail.cpp deleted file mode 100755 index 0348c2cea89..00000000000 --- a/cmake/CheckCXX11Features/cxx11-test-sizeof_member_fail.cpp +++ /dev/null @@ -1,9 +0,0 @@ -struct foo { - int baz; - double bar; -}; - -int main(void) -{ - return (sizeof(foo::bar) == 4) ? 0 : 1; -} diff --git a/cmake/CheckCXX11Features/cxx11-test-static_assert.cpp b/cmake/CheckCXX11Features/cxx11-test-static_assert.cpp deleted file mode 100755 index 47c2fefb8b2..00000000000 --- a/cmake/CheckCXX11Features/cxx11-test-static_assert.cpp +++ /dev/null @@ -1,5 +0,0 @@ -int main(void) -{ - static_assert(0 < 1, "your ordering of integers is screwed"); - return 0; -} diff --git a/cmake/CheckCXX11Features/cxx11-test-static_assert_fail_compile.cpp b/cmake/CheckCXX11Features/cxx11-test-static_assert_fail_compile.cpp deleted file mode 100755 index 362fcdde9c4..00000000000 --- a/cmake/CheckCXX11Features/cxx11-test-static_assert_fail_compile.cpp +++ /dev/null @@ -1,5 +0,0 @@ -int main(void) -{ - static_assert(1 < 0, "your ordering of integers is screwed"); - return 0; -} diff --git a/cmake/CheckCXX11Features/cxx11-test-variadic_templates.cpp b/cmake/CheckCXX11Features/cxx11-test-variadic_templates.cpp deleted file mode 100755 index 4518e886fd1..00000000000 --- a/cmake/CheckCXX11Features/cxx11-test-variadic_templates.cpp +++ /dev/null @@ -1,23 +0,0 @@ -int Accumulate() -{ - return 0; -} - -template -int Accumulate(T v, Ts... vs) -{ - return v + Accumulate(vs...); -} - -template -int CountElements() -{ - return sizeof...(Is); -} - -int main() -{ - int acc = Accumulate(1, 2, 3, 4, -5); - int count = CountElements<1,2,3,4,5>(); - return ((acc == 5) && (count == 5)) ? 0 : 1; -} diff --git a/src/interfaces/python_modular/CMakeLists.txt b/src/interfaces/python_modular/CMakeLists.txt index 67517e432c4..09c58132ddc 100644 --- a/src/interfaces/python_modular/CMakeLists.txt +++ b/src/interfaces/python_modular/CMakeLists.txt @@ -13,16 +13,6 @@ ELSE() SET(TARGET_SWIGFLAGS "-builtin\;-modern\;-modernargs\;-threads") ENDIF() -# SWIG was broken for combining -builtin and -modernargs -# from v3.0.0 and until 3.0.4. This bug was fixed in -# v3.0.5. Make CMake emit an error and fail to configure. -IF((NOT "${SWIG_VERSION}" VERSION_LESS "3.0.0") AND - ("${SWIG_VERSION}" VERSION_LESS "3.0.5")) - MESSAGE(FATAL_ERROR - "This version of SWIG is broken for building Python_modular interface. Use SWIG < 3.0.0 or >= 3.0.5.") -ENDIF((NOT "${SWIG_VERSION}" VERSION_LESS "3.0.0") AND - ("${SWIG_VERSION}" VERSION_LESS "3.0.5")) - # SWIG-generated Python-wrappers fail to build # for Python >=3.5 with SWIG < 3.0.8. Make CMake # emit an error and fail to configure. @@ -33,7 +23,6 @@ IF ((NOT "${PYTHON_VERSION_STRING}" VERSION_LESS "3.5") AND ENDIF ((NOT "${PYTHON_VERSION_STRING}" VERSION_LESS "3.5") AND ("${SWIG_VERSION}" VERSION_LESS "3.0.8")) - include(CommonModularInterface) include_directories(${PYTHON_INCLUDE_PATH} ${NUMPY_INCLUDE_DIRS}) From 3162c47a31dd0fbfcfeb6601a4dc3033f29cf5fd Mon Sep 17 00:00:00 2001 From: lambday Date: Tue, 21 Feb 2017 19:26:39 +0530 Subject: [PATCH 02/24] Fixed the reset_stream bug caused by InputParser::exit_parser The method exit_parser calls the destructor of the std::thread object (assuming C++11 is present). A thread join or detatch must be performed before this. So, added a check before calling exit_parser in StreamingDenseFeatures and StreamingVwFeatures whether the parser is in running state. If it is, then added a call to end_parser() before calling exit_parser(), which does the thread join. --- src/shogun/features/streaming/StreamingDenseFeatures.cpp | 2 ++ src/shogun/features/streaming/StreamingVwFeatures.cpp | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/shogun/features/streaming/StreamingDenseFeatures.cpp b/src/shogun/features/streaming/StreamingDenseFeatures.cpp index 1db8f72ac5a..279dece6a55 100644 --- a/src/shogun/features/streaming/StreamingDenseFeatures.cpp +++ b/src/shogun/features/streaming/StreamingDenseFeatures.cpp @@ -67,6 +67,8 @@ template void CStreamingDenseFeatures::reset_stream() if (seekable) { ((CStreamingFileFromDenseFeatures*)working_file)->reset_stream(); + if (parser.is_running()) + parser.end_parser(); parser.exit_parser(); parser.init(working_file, has_labels, 1); parser.set_free_vector_after_release(false); diff --git a/src/shogun/features/streaming/StreamingVwFeatures.cpp b/src/shogun/features/streaming/StreamingVwFeatures.cpp index 43a9c195f3a..9c00f9e1a23 100644 --- a/src/shogun/features/streaming/StreamingVwFeatures.cpp +++ b/src/shogun/features/streaming/StreamingVwFeatures.cpp @@ -66,6 +66,8 @@ void CStreamingVwFeatures::reset_stream() if (working_file->is_seekable()) { working_file->reset_stream(); + if (parser.is_running()) + parser.end_parser(); parser.exit_parser(); parser.init(working_file, has_labels, parser.get_ring_size()); parser.set_free_vector_after_release(false); From b4e46fbcba7921177393fcbcce08b02b2622e7bd Mon Sep 17 00:00:00 2001 From: Heiko Strathmann Date: Tue, 21 Feb 2017 14:15:26 +0000 Subject: [PATCH 03/24] Re-enable the failing linear time mmd unit tests This reverts commit 1560b3b39e214be4cd066c6ca4005496dc9a3354. --- tests/unit/statistical_testing/LinearTimeMMD_unittest.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/statistical_testing/LinearTimeMMD_unittest.cc b/tests/unit/statistical_testing/LinearTimeMMD_unittest.cc index 0acf63e3ef5..c74b92e4189 100644 --- a/tests/unit/statistical_testing/LinearTimeMMD_unittest.cc +++ b/tests/unit/statistical_testing/LinearTimeMMD_unittest.cc @@ -311,7 +311,7 @@ TEST(LinearTimeMMD, compute_variance_null) EXPECT_NEAR(var, 0.0022330284118652344, 1E-10); } -TEST(LinearTimeMMD, DISABLED_perform_test_permutation_biased_full) +TEST(LinearTimeMMD, perform_test_permutation_biased_full) { const index_t m=20; const index_t n=30; @@ -387,7 +387,7 @@ TEST(LinearTimeMMD, perform_test_permutation_unbiased_full) EXPECT_NEAR(p_value, 0.0, 1E-10); } -TEST(LinearTimeMMD, DISABLED_perform_test_permutation_unbiased_incomplete) +TEST(LinearTimeMMD, perform_test_permutation_unbiased_incomplete) { const index_t m=20; const index_t n=20; From 7a54aba004597ea7ded7344b789d7c173d56eb3d Mon Sep 17 00:00:00 2001 From: Heiko Strathmann Date: Tue, 21 Feb 2017 16:42:51 +0000 Subject: [PATCH 04/24] removing permutation test for linear time MMD It does not make sense to run this as there is the Gaussian approximation of the null distribution. Removing unit tests (were buggy), changing default behaviour, and throwing an error if used, --- .../statistical_testing/LinearTimeMMD.cpp | 6 + .../statistical_testing/StreamingMMD.cpp | 2 +- .../LinearTimeMMD_unittest.cc | 114 ------------------ 3 files changed, 7 insertions(+), 115 deletions(-) diff --git a/src/shogun/statistical_testing/LinearTimeMMD.cpp b/src/shogun/statistical_testing/LinearTimeMMD.cpp index c1e9fac7de5..c16e8eca7f3 100644 --- a/src/shogun/statistical_testing/LinearTimeMMD.cpp +++ b/src/shogun/statistical_testing/LinearTimeMMD.cpp @@ -116,6 +116,12 @@ float64_t CLinearTimeMMD::compute_p_value(float64_t statistic) result = 1.0 - CStatistics::normal_cdf(statistic, std_dev); break; } + case NAM_PERMUTATION: + { + SG_SERROR("Null approximation via permutation does not make sense " + "for linear time MMD. Use the Gaussian approximation instead.\n"); + break; + } default: { result = CHypothesisTest::compute_p_value(statistic); diff --git a/src/shogun/statistical_testing/StreamingMMD.cpp b/src/shogun/statistical_testing/StreamingMMD.cpp index a16b6dc9d3d..f452b85c7a9 100644 --- a/src/shogun/statistical_testing/StreamingMMD.cpp +++ b/src/shogun/statistical_testing/StreamingMMD.cpp @@ -87,7 +87,7 @@ CStreamingMMD::Self::Self(CStreamingMMD& cmmd) : owner(cmmd), use_gpu(false), num_null_samples(250), statistic_type(ST_UNBIASED_FULL), variance_estimation_method(VEM_DIRECT), - null_approximation_method(NAM_PERMUTATION), + null_approximation_method(NAM_MMD1_GAUSSIAN), statistic_job(nullptr), variance_job(nullptr) { } diff --git a/tests/unit/statistical_testing/LinearTimeMMD_unittest.cc b/tests/unit/statistical_testing/LinearTimeMMD_unittest.cc index c74b92e4189..0cbb53836b4 100644 --- a/tests/unit/statistical_testing/LinearTimeMMD_unittest.cc +++ b/tests/unit/statistical_testing/LinearTimeMMD_unittest.cc @@ -311,120 +311,6 @@ TEST(LinearTimeMMD, compute_variance_null) EXPECT_NEAR(var, 0.0022330284118652344, 1E-10); } -TEST(LinearTimeMMD, perform_test_permutation_biased_full) -{ - const index_t m=20; - const index_t n=30; - const index_t dim=3; - - // use fixed seed - sg_rand->set_seed(12345); - - float64_t difference=0.5; - - // streaming data generator for mean shift distributions - auto gen_p=new CMeanShiftDataGenerator(0, dim, 0); - auto gen_q=new CMeanShiftDataGenerator(difference, dim, 0); - - // shoguns kernel width is different - float64_t sigma=2; - float64_t sq_sigma_twice=sigma*sigma*2; - CGaussianKernel* kernel=new CGaussianKernel(10, sq_sigma_twice); - - // create MMD instance, convienience constructor - auto mmd=some(gen_p, gen_q); - mmd->set_num_samples_p(m); - mmd->set_num_samples_q(n); - mmd->set_num_blocks_per_burst(1000); - mmd->set_kernel(kernel); - - index_t num_null_samples=10; - mmd->set_num_null_samples(num_null_samples); - mmd->set_null_approximation_method(NAM_PERMUTATION); - - // compute p-value using permutation for null distribution and - // assert against local machine computed result - mmd->set_statistic_type(ST_BIASED_FULL); - float64_t p_value=mmd->compute_p_value(mmd->compute_statistic()); - EXPECT_NEAR(p_value, 0.0, 1E-10); -} - -TEST(LinearTimeMMD, perform_test_permutation_unbiased_full) -{ - const index_t m=20; - const index_t n=30; - const index_t dim=3; - - // use fixed seed - sg_rand->set_seed(12345); - - float64_t difference=0.5; - - // streaming data generator for mean shift distributions - auto gen_p=new CMeanShiftDataGenerator(0, dim, 0); - auto gen_q=new CMeanShiftDataGenerator(difference, dim, 0); - - // shoguns kernel width is different - float64_t sigma=2; - float64_t sq_sigma_twice=sigma*sigma*2; - CGaussianKernel* kernel=new CGaussianKernel(10, sq_sigma_twice); - - // create MMD instance, convienience constructor - auto mmd=some(gen_p, gen_q); - mmd->set_num_samples_p(m); - mmd->set_num_samples_q(n); - mmd->set_num_blocks_per_burst(1000); - mmd->set_kernel(kernel); - - index_t num_null_samples=10; - mmd->set_num_null_samples(num_null_samples); - mmd->set_null_approximation_method(NAM_PERMUTATION); - - // compute p-value using permutation for null distribution and - // assert against local machine computed result - mmd->set_statistic_type(ST_UNBIASED_FULL); - float64_t p_value=mmd->compute_p_value(mmd->compute_statistic()); - EXPECT_NEAR(p_value, 0.0, 1E-10); -} - -TEST(LinearTimeMMD, perform_test_permutation_unbiased_incomplete) -{ - const index_t m=20; - const index_t n=20; - const index_t dim=3; - - // use fixed seed - sg_rand->set_seed(12345); - - float64_t difference=0.5; - - // streaming data generator for mean shift distributions - auto gen_p=new CMeanShiftDataGenerator(0, dim, 0); - auto gen_q=new CMeanShiftDataGenerator(difference, dim, 0); - - // shoguns kernel width is different - float64_t sigma=2; - float64_t sq_sigma_twice=sigma*sigma*2; - CGaussianKernel* kernel=new CGaussianKernel(10, sq_sigma_twice); - - // create MMD instance, convienience constructor - auto mmd=some(gen_p, gen_q); - mmd->set_num_samples_p(m); - mmd->set_num_samples_q(n); - mmd->set_num_blocks_per_burst(1000); - mmd->set_kernel(kernel); - - index_t num_null_samples=10; - mmd->set_num_null_samples(num_null_samples); - mmd->set_null_approximation_method(NAM_PERMUTATION); - - // compute p-value using permutation for null distribution and - // assert against local machine computed result - mmd->set_statistic_type(ST_UNBIASED_INCOMPLETE); - float64_t p_value=mmd->compute_p_value(mmd->compute_statistic()); - EXPECT_NEAR(p_value, 0.59999999999999998, 1E-10); -} - TEST(LinearTimeMMD, perform_test_gaussian_biased_full) { const index_t m=20; From 4d1a9b080217c026fc8961df2db90426e209fd52 Mon Sep 17 00:00:00 2001 From: Heiko Strathmann Date: Tue, 21 Feb 2017 18:06:28 +0000 Subject: [PATCH 05/24] get rid of size_t ambiguity and openmp loop std violations Via replacing key quantities with index_t, and some hard casting. --- .../statistical_testing/StreamingMMD.cpp | 20 ++++++++--------- .../internals/ComputationManager.cpp | 2 +- .../internals/DataManager.cpp | 22 +++++++++---------- .../internals/DataManager.h | 12 +++++----- .../internals/KernelManager.cpp | 18 +++++++-------- .../internals/KernelManager.h | 10 ++++----- .../internals/mmd/ComputeMMD.h | 4 ++-- .../internals/mmd/CrossValidationMMD.h | 4 ++-- .../internals/mmd/PermutationMMD.h | 4 ++-- .../internals/mmd/VarianceH1.h | 4 ++-- .../internals/MaxCrossValidation.cpp | 8 +++---- .../kernelselection/internals/MaxMeasure.cpp | 6 ++--- .../internals/MaxTestPower.cpp | 4 ++-- .../internals/MedianHeuristic.cpp | 10 ++++----- .../internals/WeightedMaxMeasure.cpp | 4 ++-- .../internals/MultiKernelMMD_unittest.cc | 6 ++--- 16 files changed, 69 insertions(+), 69 deletions(-) diff --git a/src/shogun/statistical_testing/StreamingMMD.cpp b/src/shogun/statistical_testing/StreamingMMD.cpp index f452b85c7a9..d26abf72202 100644 --- a/src/shogun/statistical_testing/StreamingMMD.cpp +++ b/src/shogun/statistical_testing/StreamingMMD.cpp @@ -135,7 +135,7 @@ void CStreamingMMD::Self::merge_samples(NextSamples& next_burst, std::vectorget_kernel_type()!=K_CUSTOM, "Underlying kernel cannot be custom!\n"); cm.num_data(blocks.size()); #pragma omp parallel for - for (size_t i=0; i, SGMatrix > CStreamingMMD::Self::comput REQUIRE(kernel_selection_mgr.num_kernels()>0, "No kernels specified for kernel learning! " "Please add kernels using add_kernel() method!\n"); - const size_t num_kernels=kernel_selection_mgr.num_kernels(); + const auto num_kernels=kernel_selection_mgr.num_kernels(); SGVector statistic(num_kernels); SGMatrix Q(num_kernels, num_kernels); @@ -279,19 +279,19 @@ std::pair, SGMatrix > CStreamingMMD::Self::comput std::vector > mmds(num_kernels); while (!next_burst.empty()) { - const size_t num_blocks=next_burst.num_blocks(); + const auto num_blocks=next_burst.num_blocks(); REQUIRE(num_blocks%2==0, "The number of blocks per burst (%d this burst) has to be even!\n", num_blocks); merge_samples(next_burst, blocks); std::for_each(blocks.begin(), blocks.end(), [](CFeatures* ptr) { SG_REF(ptr); }); - for (size_t k=0; k, SGMatrix > CStreamingMMD::Self::comput } std::for_each(blocks.begin(), blocks.end(), [](CFeatures* ptr) { SG_UNREF(ptr); }); blocks.resize(0); - for (size_t i=0; im_num_samples; } -const index_t DataManager::num_samples_at(size_t i) const +const index_t DataManager::num_samples_at(index_t i) const { SG_SDEBUG("Entering!\n"); - REQUIRE(i0); bool same=false; EDistanceType distance_type=D_UNKNOWN; - for (size_t i=0; i(kernel_at(i)); if (shift_invariant_kernel!=nullptr) @@ -200,7 +200,7 @@ CDistance* KernelManager::get_distance_instance() const void KernelManager::set_precomputed_distance(CCustomDistance* distance) const { - for (size_t i=0; i(kernel); @@ -213,7 +213,7 @@ void KernelManager::set_precomputed_distance(CCustomDistance* distance) const void KernelManager::unset_precomputed_distance() const { - for (size_t i=0; i(kernel); diff --git a/src/shogun/statistical_testing/internals/KernelManager.h b/src/shogun/statistical_testing/internals/KernelManager.h index 51a398bf47c..f737e13b03f 100644 --- a/src/shogun/statistical_testing/internals/KernelManager.h +++ b/src/shogun/statistical_testing/internals/KernelManager.h @@ -54,14 +54,14 @@ class KernelManager explicit KernelManager(index_t num_kernels); ~KernelManager(); - InitPerKernel kernel_at(size_t i); - CKernel* kernel_at(size_t i) const; + InitPerKernel kernel_at(index_t i); + CKernel* kernel_at(index_t i) const; void push_back(CKernel* kernel); - const size_t num_kernels() const; + const index_t num_kernels() const; - void precompute_kernel_at(size_t i); - void restore_kernel_at(size_t i); + void precompute_kernel_at(index_t i); + void restore_kernel_at(index_t i); void clear(); bool same_distance_type() const; diff --git a/src/shogun/statistical_testing/internals/mmd/ComputeMMD.h b/src/shogun/statistical_testing/internals/mmd/ComputeMMD.h index ab599a2f5a8..8c869d8cd57 100644 --- a/src/shogun/statistical_testing/internals/mmd/ComputeMMD.h +++ b/src/shogun/statistical_testing/internals/mmd/ComputeMMD.h @@ -118,7 +118,7 @@ struct ComputeMMD { for (auto i=j; ikernel(i, j); add_term_lower(terms[k], kernel, i, j); @@ -127,7 +127,7 @@ struct ComputeMMD } SGVector result(kernel_mgr.num_kernels()); - for (size_t k=0; k= %d*%d = %d!\n", m_rejections.num_rows, m_num_runs, m_num_folds, m_num_runs*m_num_folds); - REQUIRE(size_t(m_rejections.num_cols)==kernel_mgr.num_kernels(), + REQUIRE(m_rejections.num_cols==kernel_mgr.num_kernels(), "Number of columns in the measure matrix (was %d), has to equal to the nunber of kernels (%d)!\n", m_rejections.num_cols, kernel_mgr.num_kernels()); @@ -85,7 +85,7 @@ struct CrossValidationMMD : PermutationMMD SGVector null_samples(m_num_null_samples); SGVector precomputed_km(size*(size+1)/2); - for (size_t k=0; k null_samples(m_num_null_samples, kernel_mgr.num_kernels()); SGVector km(size*(size+1)/2); - for (size_t k=0; k result(kernel_mgr.num_kernels()); SGVector km(size*(size+1)/2); - for (size_t k=0; k result(kernel_mgr.num_kernels()); SelfAdjointPrecomputedKernel kernel_functor(SGVector(size*(size+1)/2)); - for (size_t k=0; k result(kernel_mgr.num_kernels()); SelfAdjointPrecomputedKernel kernel_functor(SGVector(size*(size+1)/2)); - for (size_t k=0; k(estimator); if (quadratic_time_mmd) @@ -116,7 +116,7 @@ void MaxCrossValidation::compute_measures() auto samples_p_and_q=quadratic_time_mmd->get_p_and_q(); SG_REF(samples_p_and_q); - for (size_t k=0; kinit(samples_p_and_q, samples_p_and_q); @@ -124,7 +124,7 @@ void MaxCrossValidation::compute_measures() compute(kernel_mgr); - for (size_t k=0; kremove_lhs_and_rhs(); @@ -141,7 +141,7 @@ void MaxCrossValidation::compute_measures() for (auto j=0; jset_kernel(kernel); diff --git a/src/shogun/statistical_testing/kernelselection/internals/MaxMeasure.cpp b/src/shogun/statistical_testing/kernelselection/internals/MaxMeasure.cpp index d0748e86230..8e1e721d9a3 100644 --- a/src/shogun/statistical_testing/kernelselection/internals/MaxMeasure.cpp +++ b/src/shogun/statistical_testing/kernelselection/internals/MaxMeasure.cpp @@ -80,8 +80,8 @@ void MaxMeasure::compute_measures() { init_measures(); auto existing_kernel=estimator->get_kernel(); - const size_t num_kernels=kernel_mgr.num_kernels(); - for (size_t i=0; iset_kernel(kernel); @@ -96,7 +96,7 @@ void MaxMeasure::compute_measures() CKernel* MaxMeasure::select_kernel() { compute_measures(); - ASSERT(size_t(measures.size())==kernel_mgr.num_kernels()); + ASSERT(measures.size()==kernel_mgr.num_kernels()); auto max_element=std::max_element(measures.vector, measures.vector+measures.vlen); auto max_idx=std::distance(measures.vector, max_element); SG_SDEBUG("Selected kernel at %d position!\n", max_idx); diff --git a/src/shogun/statistical_testing/kernelselection/internals/MaxTestPower.cpp b/src/shogun/statistical_testing/kernelselection/internals/MaxTestPower.cpp index cb1635bfc43..645b0cdad8c 100644 --- a/src/shogun/statistical_testing/kernelselection/internals/MaxTestPower.cpp +++ b/src/shogun/statistical_testing/kernelselection/internals/MaxTestPower.cpp @@ -57,11 +57,11 @@ void MaxTestPower::compute_measures() const auto m=estimator->get_num_samples_p(); const auto n=estimator->get_num_samples_q(); auto existing_kernel=estimator->get_kernel(); - const size_t num_kernels=kernel_mgr.num_kernels(); + const auto num_kernels=kernel_mgr.num_kernels(); auto streaming_mmd=dynamic_cast(estimator); if (streaming_mmd) { - for (size_t i=0; iset_kernel(kernel); diff --git a/src/shogun/statistical_testing/kernelselection/internals/MedianHeuristic.cpp b/src/shogun/statistical_testing/kernelselection/internals/MedianHeuristic.cpp index d7e98d69f04..e570884bc1f 100644 --- a/src/shogun/statistical_testing/kernelselection/internals/MedianHeuristic.cpp +++ b/src/shogun/statistical_testing/kernelselection/internals/MedianHeuristic.cpp @@ -45,7 +45,7 @@ using namespace internal; MedianHeuristic::MedianHeuristic(KernelManager& km, CMMD* est) : KernelSelection(km, est), distance(nullptr) { - for (size_t i=0; iget_kernel_type()==K_GAUSSIAN, "The underlying kernel has to be a GaussianKernel (was %s)!\n", @@ -75,7 +75,7 @@ void MedianHeuristic::compute_measures() "Distance matrix is supposed to be a square matrix (was of dimension %dX%d)!\n", distance->get_num_vec_lhs(), distance->get_num_vec_rhs()); measures=SGVector((n*(n-1))/2); - size_t write_idx=0; + index_t write_idx=0; for (auto j=0; j(num_kernels); - for (size_t i=0; i(kernel_mgr.kernel_at(i)); measures[i]=CMath::abs(kernel->get_width()-median_distance); } - size_t kernel_idx=std::distance(measures.data(), std::min_element(measures.data(), measures.data()+measures.size())); + auto kernel_idx=(index_t)std::distance(measures.data(), std::min_element(measures.data(), measures.data()+measures.size())); SG_SDEBUG("Selected kernel at %d position!\n", kernel_idx); return kernel_mgr.kernel_at(kernel_idx); } diff --git a/src/shogun/statistical_testing/kernelselection/internals/WeightedMaxMeasure.cpp b/src/shogun/statistical_testing/kernelselection/internals/WeightedMaxMeasure.cpp index 4ce92028c0d..41acc8c7f91 100644 --- a/src/shogun/statistical_testing/kernelselection/internals/WeightedMaxMeasure.cpp +++ b/src/shogun/statistical_testing/kernelselection/internals/WeightedMaxMeasure.cpp @@ -52,11 +52,11 @@ WeightedMaxMeasure::~WeightedMaxMeasure() void WeightedMaxMeasure::compute_measures() { MaxMeasure::compute_measures(); - const size_t num_kernels=kernel_mgr.num_kernels(); + const auto num_kernels=kernel_mgr.num_kernels(); if (Q.num_rows!=num_kernels || Q.num_cols!=num_kernels) Q=SGMatrix(num_kernels, num_kernels); std::fill(Q.data(), Q.data()+Q.size(), 0); - for (size_t i=0; i ref(kernel_mgr.num_kernels()); - for (size_t i=0; iinit(feats_p_and_q, feats_p_and_q); @@ -170,7 +170,7 @@ TEST(MultiKernelMMD, unbiased_full) SG_REF(feats_p_and_q); SGVector ref(kernel_mgr.num_kernels()); - for (size_t i=0; iinit(feats_p_and_q, feats_p_and_q); @@ -235,7 +235,7 @@ TEST(MultiKernelMMD, unbiased_incomplete) SG_REF(feats_p_and_q); SGVector ref(kernel_mgr.num_kernels()); - for (size_t i=0; iinit(feats_p_and_q, feats_p_and_q); From d546a96abc2e4371e7565a6c25d4a60908b9aabb Mon Sep 17 00:00:00 2001 From: Heiko Strathmann Date: Tue, 21 Feb 2017 22:10:16 +0000 Subject: [PATCH 06/24] add greatest common divisor --- src/shogun/mathematics/Math.h | 26 +++++++++++++++++++++++++ tests/unit/mathematics/Math_unittest.cc | 16 ++++++++++++++- 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/src/shogun/mathematics/Math.h b/src/shogun/mathematics/Math.h index 4ccffd3d89f..021b4bb214a 100644 --- a/src/shogun/mathematics/Math.h +++ b/src/shogun/mathematics/Math.h @@ -1130,6 +1130,32 @@ class CMath : public CSGObject } //@} + /** Implements the greatest common divisor (gcd) via modulo operations. + * Requires that either a>0 and b>=0 or vice versa. + * + * @param a first number + * @param b second number + * @return gcd between the two numbers + */ + static int32_t gcd(int32_t a, int32_t b) + { + REQUIRE((a>=0 && b>0) || (b>=0 && a>0), "gcd(%d,%d) is not defined.\n", + a, b); + + if (1 == a || 1 == b) + return 1; + + while (0 < a && 0 < b) + { + if (a > b) + a %= b; + else + b %= a; + } + + return 0 == a ? b : a; + } + /** Permute randomly the elements of the vector. If provided, use the * random object to generate the permutations. * @param v the vector to permute. diff --git a/tests/unit/mathematics/Math_unittest.cc b/tests/unit/mathematics/Math_unittest.cc index ed6934a7e2f..f8684cc579b 100644 --- a/tests/unit/mathematics/Math_unittest.cc +++ b/tests/unit/mathematics/Math_unittest.cc @@ -512,4 +512,18 @@ TEST(CMath, dot) float64_t sgdot_val = CMath::dot(a.vector,a.vector, a.vlen); EXPECT_NEAR(dot_val, sgdot_val, 1e-9); -} \ No newline at end of file +} + +TEST(CMath, gcd) +{ + EXPECT_EQ(CMath::gcd(12,8), 4); + EXPECT_EQ(CMath::gcd(18,27), 9); + EXPECT_EQ(CMath::gcd(1,1), 1); + EXPECT_EQ(CMath::gcd(1,2), 1); + EXPECT_EQ(CMath::gcd(1,0), 1); + EXPECT_EQ(CMath::gcd(0,1), 1); + EXPECT_THROW(CMath::gcd(0,0), ShogunException); + EXPECT_THROW(CMath::gcd(1,-1), ShogunException); + EXPECT_THROW(CMath::gcd(-1,1), ShogunException); + +} From 8fd8294b261407488478c117ab24e23d3d70be74 Mon Sep 17 00:00:00 2001 From: Heiko Strathmann Date: Tue, 21 Feb 2017 22:10:52 +0000 Subject: [PATCH 07/24] use CMath::gcd rather than C++17 and std::function --- src/shogun/statistical_testing/internals/DataManager.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/shogun/statistical_testing/internals/DataManager.cpp b/src/shogun/statistical_testing/internals/DataManager.cpp index b3efd78759e..1a1a30d0580 100644 --- a/src/shogun/statistical_testing/internals/DataManager.cpp +++ b/src/shogun/statistical_testing/internals/DataManager.cpp @@ -80,12 +80,8 @@ index_t DataManager::get_min_blocksize() const else { index_t divisor=0; - std::function gcd=[&gcd](index_t m, index_t n) - { - return n==0?m:gcd(n, m%n); - }; for (size_t i=0; im_num_samples); + divisor=CMath::gcd(divisor, fetchers[i]->m_num_samples); min_blocksize=get_num_samples()/divisor; } SG_SDEBUG("min blocksize is %d!", min_blocksize); From 23aaa318810f956541345ee593679e82ff32a3e2 Mon Sep 17 00:00:00 2001 From: Viktor Gal Date: Wed, 22 Feb 2017 06:23:16 +0800 Subject: [PATCH 08/24] Be consistent with the override marker include missing numeric header make the libshogun target depend on headers as well --- src/shogun/CMakeLists.txt | 2 +- .../kernelselection/internals/MaxCrossValidation.h | 8 ++++---- .../kernelselection/internals/MedianHeuristic.h | 8 ++++---- .../unit/statistical_testing/internals/Block_unittest.cc | 1 + .../statistical_testing/internals/DataFetcher_unittest.cc | 1 + .../statistical_testing/internals/DataManager_unittest.cc | 1 + .../internals/FeaturesUtil_unittest.cc | 1 + .../internals/StreamingDataFetcher_unittest.cc | 1 + .../internals/WithinBlockPermutation_unittest.cc | 1 + 9 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/shogun/CMakeLists.txt b/src/shogun/CMakeLists.txt index 698af0c23ea..d712d4160e5 100644 --- a/src/shogun/CMakeLists.txt +++ b/src/shogun/CMakeLists.txt @@ -30,7 +30,7 @@ if (MSVC AND (BUILD_EXAMPLES OR BUILD_META_EXAMPLES)) endif() # add target to compile the libshogun sources -add_library(libshogun OBJECT ${LIBSHOGUN_SRC} ${CMAKE_CURRENT_BINARY_DIR}/lib/config.h) +add_library(libshogun OBJECT ${LIBSHOGUN_SRC} ${LIBSHOGUN_HEADERS} ${CMAKE_CURRENT_BINARY_DIR}/lib/config.h) set_property(TARGET libshogun PROPERTY POSITION_INDEPENDENT_CODE TRUE) IF (SANITIZER_FLAGS) set_property(libshogun PROPERTY COMPILE_FLAGS ${SANITIZER_FLAGS}) diff --git a/src/shogun/statistical_testing/kernelselection/internals/MaxCrossValidation.h b/src/shogun/statistical_testing/kernelselection/internals/MaxCrossValidation.h index 4dda6a213bd..78edd5f9778 100644 --- a/src/shogun/statistical_testing/kernelselection/internals/MaxCrossValidation.h +++ b/src/shogun/statistical_testing/kernelselection/internals/MaxCrossValidation.h @@ -53,11 +53,11 @@ class MaxCrossValidation : public KernelSelection ~MaxCrossValidation(); MaxCrossValidation& operator=(const MaxCrossValidation& other)=delete; virtual CKernel* select_kernel() override; - virtual SGVector get_measure_vector(); - virtual SGMatrix get_measure_matrix(); + virtual SGVector get_measure_vector() override; + virtual SGMatrix get_measure_matrix() override; protected: - virtual void init_measures(); - virtual void compute_measures(); + virtual void init_measures() override; + virtual void compute_measures() override; const index_t num_runs; const index_t num_folds; const float64_t alpha; diff --git a/src/shogun/statistical_testing/kernelselection/internals/MedianHeuristic.h b/src/shogun/statistical_testing/kernelselection/internals/MedianHeuristic.h index a59d457a647..f621dd76b5a 100644 --- a/src/shogun/statistical_testing/kernelselection/internals/MedianHeuristic.h +++ b/src/shogun/statistical_testing/kernelselection/internals/MedianHeuristic.h @@ -55,11 +55,11 @@ class MedianHeuristic : public KernelSelection ~MedianHeuristic(); MedianHeuristic& operator=(const MedianHeuristic& other)=delete; virtual CKernel* select_kernel() override; - virtual SGVector get_measure_vector(); - virtual SGMatrix get_measure_matrix(); + virtual SGVector get_measure_vector() override; + virtual SGMatrix get_measure_matrix() override; protected: - virtual void init_measures(); - virtual void compute_measures(); + virtual void init_measures() override; + virtual void compute_measures() override; std::shared_ptr distance; SGVector measures; int32_t n; diff --git a/tests/unit/statistical_testing/internals/Block_unittest.cc b/tests/unit/statistical_testing/internals/Block_unittest.cc index cbedeb90720..7d1ddee92c4 100644 --- a/tests/unit/statistical_testing/internals/Block_unittest.cc +++ b/tests/unit/statistical_testing/internals/Block_unittest.cc @@ -28,6 +28,7 @@ * either expressed or implied, of the Shogun Development Team. */ +#include #include #include #include diff --git a/tests/unit/statistical_testing/internals/DataFetcher_unittest.cc b/tests/unit/statistical_testing/internals/DataFetcher_unittest.cc index d68ac582c88..cae0b9d2307 100644 --- a/tests/unit/statistical_testing/internals/DataFetcher_unittest.cc +++ b/tests/unit/statistical_testing/internals/DataFetcher_unittest.cc @@ -29,6 +29,7 @@ */ #include +#include #include #include #include diff --git a/tests/unit/statistical_testing/internals/DataManager_unittest.cc b/tests/unit/statistical_testing/internals/DataManager_unittest.cc index fab238474ae..c79793596c5 100644 --- a/tests/unit/statistical_testing/internals/DataManager_unittest.cc +++ b/tests/unit/statistical_testing/internals/DataManager_unittest.cc @@ -28,6 +28,7 @@ * either expressed or implied, of the Shogun Development Team. */ +#include #include #include #include diff --git a/tests/unit/statistical_testing/internals/FeaturesUtil_unittest.cc b/tests/unit/statistical_testing/internals/FeaturesUtil_unittest.cc index 5d48e79c007..140417075e3 100644 --- a/tests/unit/statistical_testing/internals/FeaturesUtil_unittest.cc +++ b/tests/unit/statistical_testing/internals/FeaturesUtil_unittest.cc @@ -29,6 +29,7 @@ */ #include +#include #include #include #include diff --git a/tests/unit/statistical_testing/internals/StreamingDataFetcher_unittest.cc b/tests/unit/statistical_testing/internals/StreamingDataFetcher_unittest.cc index d6b4146af03..a708a41d5da 100644 --- a/tests/unit/statistical_testing/internals/StreamingDataFetcher_unittest.cc +++ b/tests/unit/statistical_testing/internals/StreamingDataFetcher_unittest.cc @@ -29,6 +29,7 @@ */ #include +#include #include #include #include diff --git a/tests/unit/statistical_testing/internals/WithinBlockPermutation_unittest.cc b/tests/unit/statistical_testing/internals/WithinBlockPermutation_unittest.cc index eb568aa77b6..c65ea42df3d 100644 --- a/tests/unit/statistical_testing/internals/WithinBlockPermutation_unittest.cc +++ b/tests/unit/statistical_testing/internals/WithinBlockPermutation_unittest.cc @@ -29,6 +29,7 @@ */ #include +#include #include #include #include From 905bd7e72e319c4a3e1817d5359fa1479c9eb893 Mon Sep 17 00:00:00 2001 From: lambday Date: Wed, 22 Feb 2017 10:40:36 +0530 Subject: [PATCH 09/24] fix a memory leak in multikernel mmd --- src/shogun/statistical_testing/TwoDistributionTest.cpp | 1 + src/shogun/statistical_testing/internals/KernelManager.cpp | 3 +++ .../statistical_testing/internals/MultiKernelMMD_unittest.cc | 3 +++ 3 files changed, 7 insertions(+) diff --git a/src/shogun/statistical_testing/TwoDistributionTest.cpp b/src/shogun/statistical_testing/TwoDistributionTest.cpp index 68698c8bb3e..b3efeca6b7d 100644 --- a/src/shogun/statistical_testing/TwoDistributionTest.cpp +++ b/src/shogun/statistical_testing/TwoDistributionTest.cpp @@ -130,6 +130,7 @@ CCustomDistance* CTwoDistributionTest::compute_distance(CDistance* distance) CCustomDistance* CTwoDistributionTest::compute_joint_distance(CDistance* distance) { + REQUIRE(distance!=nullptr, "Distance instance cannot be NULL!\n"); auto& data_mgr=get_data_mgr(); bool is_blockwise=data_mgr.is_blockwise(); data_mgr.set_blockwise(false); diff --git a/src/shogun/statistical_testing/internals/KernelManager.cpp b/src/shogun/statistical_testing/internals/KernelManager.cpp index f4c1f9f272d..dfee576285f 100644 --- a/src/shogun/statistical_testing/internals/KernelManager.cpp +++ b/src/shogun/statistical_testing/internals/KernelManager.cpp @@ -200,12 +200,14 @@ CDistance* KernelManager::get_distance_instance() const void KernelManager::set_precomputed_distance(CCustomDistance* distance) const { + REQUIRE(distance!=nullptr, "Distance instance cannot be null!\n"); for (auto i=0; i(kernel); REQUIRE(shift_inv_kernel!=nullptr, "Kernel instance (was %s) must be of CShiftInvarintKernel type!\n", kernel->get_name()); shift_inv_kernel->m_precomputed_distance=distance; + SG_REF(shift_inv_kernel->m_precomputed_distance); shift_inv_kernel->num_lhs=distance->get_num_vec_lhs(); shift_inv_kernel->num_rhs=distance->get_num_vec_rhs(); } @@ -218,6 +220,7 @@ void KernelManager::unset_precomputed_distance() const CKernel* kernel=kernel_at(i); CShiftInvariantKernel* shift_inv_kernel=dynamic_cast(kernel); REQUIRE(shift_inv_kernel!=nullptr, "Kernel instance (was %s) must be of CShiftInvarintKernel type!\n", kernel->get_name()); + SG_UNREF(shift_inv_kernel->m_precomputed_distance); shift_inv_kernel->m_precomputed_distance=nullptr; shift_inv_kernel->num_lhs=0; shift_inv_kernel->num_rhs=0; diff --git a/tests/unit/statistical_testing/internals/MultiKernelMMD_unittest.cc b/tests/unit/statistical_testing/internals/MultiKernelMMD_unittest.cc index a0d911ad6e5..68eb33cd7b1 100644 --- a/tests/unit/statistical_testing/internals/MultiKernelMMD_unittest.cc +++ b/tests/unit/statistical_testing/internals/MultiKernelMMD_unittest.cc @@ -88,6 +88,7 @@ TEST(MultiKernelMMD, biased_full) kernel_mgr.kernel_at(i)=new CGaussianKernel(10, pow(2, sigma)); auto distance=kernel_mgr.get_distance_instance(); kernel_mgr.set_precomputed_distance(test->compute_joint_distance(distance)); + SG_UNREF(distance); ComputeMMD tester; tester.m_n_x=m; @@ -153,6 +154,7 @@ TEST(MultiKernelMMD, unbiased_full) kernel_mgr.kernel_at(i)=new CGaussianKernel(10, pow(2, sigma)); auto distance=kernel_mgr.get_distance_instance(); kernel_mgr.set_precomputed_distance(test->compute_joint_distance(distance)); + SG_UNREF(distance); ComputeMMD tester; tester.m_n_x=m; @@ -218,6 +220,7 @@ TEST(MultiKernelMMD, unbiased_incomplete) kernel_mgr.kernel_at(i)=new CGaussianKernel(10, pow(2, sigma)); auto distance=kernel_mgr.get_distance_instance(); kernel_mgr.set_precomputed_distance(test->compute_joint_distance(distance)); + SG_UNREF(distance); ComputeMMD tester; tester.m_n_x=m; From 97df8fb006faadc9a6f7bda564ef919ca38c927c Mon Sep 17 00:00:00 2001 From: Heiko Strathmann Date: Wed, 22 Feb 2017 11:12:10 +0000 Subject: [PATCH 10/24] doc fix for resize_vector and unit test --- src/shogun/lib/SGVector.h | 3 +-- tests/unit/lib/SGVector_unittest.cc | 25 +++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/shogun/lib/SGVector.h b/src/shogun/lib/SGVector.h index 411d5d94503..2a72841a290 100644 --- a/src/shogun/lib/SGVector.h +++ b/src/shogun/lib/SGVector.h @@ -182,10 +182,9 @@ template class SGVector : public SGReferencedData } #ifndef SWIG // SWIG should skip this part - /** Resize vector + /** Resize vector, with zero padding * * @param n new size - * @return if resizing was successful */ void resize_vector(int32_t n); diff --git a/tests/unit/lib/SGVector_unittest.cc b/tests/unit/lib/SGVector_unittest.cc index 50fdaf9b7c9..109a6edafbd 100644 --- a/tests/unit/lib/SGVector_unittest.cc +++ b/tests/unit/lib/SGVector_unittest.cc @@ -326,3 +326,28 @@ TEST(SGVectorTest, from_eigen3_row_vector) for (int32_t i=0; i m(len); + SGVector m_ref(len); + + for (index_t i=0; i Date: Wed, 22 Feb 2017 11:25:34 +0000 Subject: [PATCH 11/24] change index_t casts of size_t to int64_t where safe --- src/shogun/statistical_testing/StreamingMMD.cpp | 4 ++-- .../internals/ComputationManager.cpp | 2 +- .../statistical_testing/internals/DataManager.cpp | 10 +++++----- .../kernelselection/internals/MedianHeuristic.cpp | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/shogun/statistical_testing/StreamingMMD.cpp b/src/shogun/statistical_testing/StreamingMMD.cpp index d26abf72202..6b7c8a74b88 100644 --- a/src/shogun/statistical_testing/StreamingMMD.cpp +++ b/src/shogun/statistical_testing/StreamingMMD.cpp @@ -135,7 +135,7 @@ void CStreamingMMD::Self::merge_samples(NextSamples& next_burst, std::vectorget_kernel_type()!=K_CUSTOM, "Underlying kernel cannot be custom!\n"); cm.num_data(blocks.size()); #pragma omp parallel for - for (index_t i=0; i<(index_t)blocks.size(); ++i) + for (int64_t i=0; i<(int64_t)blocks.size(); ++i) { try { diff --git a/src/shogun/statistical_testing/internals/ComputationManager.cpp b/src/shogun/statistical_testing/internals/ComputationManager.cpp index fc9a3c7ed90..caeea80c62b 100644 --- a/src/shogun/statistical_testing/internals/ComputationManager.cpp +++ b/src/shogun/statistical_testing/internals/ComputationManager.cpp @@ -60,7 +60,7 @@ void ComputationManager::compute_data_parallel_jobs() else { #pragma omp parallel for - for (index_t i=0; i<(index_t)data_array.size(); ++i) + for (int64_t i=0; i<(int64_t)data_array.size(); ++i) { // using a temporary vector to hold the result, because it is // cache friendly, since the original result matrix would lead diff --git a/src/shogun/statistical_testing/internals/DataManager.cpp b/src/shogun/statistical_testing/internals/DataManager.cpp index 1a1a30d0580..7278c8d1d25 100644 --- a/src/shogun/statistical_testing/internals/DataManager.cpp +++ b/src/shogun/statistical_testing/internals/DataManager.cpp @@ -147,7 +147,7 @@ void DataManager::set_num_blocks_per_burst(index_t num_blocks_per_burst) InitPerFeature DataManager::samples_at(index_t i) { SG_SDEBUG("Entering!\n"); - REQUIRE(i<(index_t)fetchers.size(), + REQUIRE(i<(int64_t)fetchers.size(), "Value of i (%d) should be between 0 and %d, inclusive!", i, fetchers.size()-1); SG_SDEBUG("Leaving!\n"); @@ -157,7 +157,7 @@ InitPerFeature DataManager::samples_at(index_t i) CFeatures* DataManager::samples_at(index_t i) const { SG_SDEBUG("Entering!\n"); - REQUIRE(i<(index_t)fetchers.size(), + REQUIRE(i<(int64_t)fetchers.size(), "Value of i (%d) should be between 0 and %d, inclusive!", i, fetchers.size()-1); SG_SDEBUG("Leaving!\n"); @@ -170,7 +170,7 @@ CFeatures* DataManager::samples_at(index_t i) const index_t& DataManager::num_samples_at(index_t i) { SG_SDEBUG("Entering!\n"); - REQUIRE(i<(index_t)fetchers.size(), + REQUIRE(i<(int64_t)fetchers.size(), "Value of i (%d) should be between 0 and %d, inclusive!", i, fetchers.size()-1); SG_SDEBUG("Leaving!\n"); @@ -180,7 +180,7 @@ index_t& DataManager::num_samples_at(index_t i) const index_t DataManager::num_samples_at(index_t i) const { SG_SDEBUG("Entering!\n"); - REQUIRE(i<(index_t)fetchers.size(), + REQUIRE(i<(int64_t)fetchers.size(), "Value of i (%d) should be between 0 and %d, inclusive!", i, fetchers.size()-1); SG_SDEBUG("Leaving!\n"); @@ -193,7 +193,7 @@ const index_t DataManager::num_samples_at(index_t i) const const index_t DataManager::blocksize_at(index_t i) const { SG_SDEBUG("Entering!\n"); - REQUIRE(i<(index_t)fetchers.size(), + REQUIRE(i<(int64_t)fetchers.size(), "Value of i (%d) should be between 0 and %d, inclusive!", i, fetchers.size()-1); SG_SDEBUG("Leaving!\n"); diff --git a/src/shogun/statistical_testing/kernelselection/internals/MedianHeuristic.cpp b/src/shogun/statistical_testing/kernelselection/internals/MedianHeuristic.cpp index e570884bc1f..96331166d8f 100644 --- a/src/shogun/statistical_testing/kernelselection/internals/MedianHeuristic.cpp +++ b/src/shogun/statistical_testing/kernelselection/internals/MedianHeuristic.cpp @@ -109,7 +109,7 @@ CKernel* MedianHeuristic::select_kernel() measures[i]=CMath::abs(kernel->get_width()-median_distance); } - auto kernel_idx=(index_t)std::distance(measures.data(), std::min_element(measures.data(), measures.data()+measures.size())); + auto kernel_idx=(int64_t)std::distance(measures.data(), std::min_element(measures.data(), measures.data()+measures.size())); SG_SDEBUG("Selected kernel at %d position!\n", kernel_idx); return kernel_mgr.kernel_at(kernel_idx); } From 4ad4fe041390b8f5f9432a6b87da1ad3e967b558 Mon Sep 17 00:00:00 2001 From: lambday Date: Wed, 22 Feb 2017 20:40:59 +0530 Subject: [PATCH 12/24] fix segfault in unit tests --- .../statistical_testing/internals/KernelManager.cpp | 2 -- .../internals/MultiKernelMMD_unittest.cc | 12 +++++++++--- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/shogun/statistical_testing/internals/KernelManager.cpp b/src/shogun/statistical_testing/internals/KernelManager.cpp index dfee576285f..af92ec25cff 100644 --- a/src/shogun/statistical_testing/internals/KernelManager.cpp +++ b/src/shogun/statistical_testing/internals/KernelManager.cpp @@ -207,7 +207,6 @@ void KernelManager::set_precomputed_distance(CCustomDistance* distance) const CShiftInvariantKernel* shift_inv_kernel=dynamic_cast(kernel); REQUIRE(shift_inv_kernel!=nullptr, "Kernel instance (was %s) must be of CShiftInvarintKernel type!\n", kernel->get_name()); shift_inv_kernel->m_precomputed_distance=distance; - SG_REF(shift_inv_kernel->m_precomputed_distance); shift_inv_kernel->num_lhs=distance->get_num_vec_lhs(); shift_inv_kernel->num_rhs=distance->get_num_vec_rhs(); } @@ -220,7 +219,6 @@ void KernelManager::unset_precomputed_distance() const CKernel* kernel=kernel_at(i); CShiftInvariantKernel* shift_inv_kernel=dynamic_cast(kernel); REQUIRE(shift_inv_kernel!=nullptr, "Kernel instance (was %s) must be of CShiftInvarintKernel type!\n", kernel->get_name()); - SG_UNREF(shift_inv_kernel->m_precomputed_distance); shift_inv_kernel->m_precomputed_distance=nullptr; shift_inv_kernel->num_lhs=0; shift_inv_kernel->num_rhs=0; diff --git a/tests/unit/statistical_testing/internals/MultiKernelMMD_unittest.cc b/tests/unit/statistical_testing/internals/MultiKernelMMD_unittest.cc index 68eb33cd7b1..82869eada48 100644 --- a/tests/unit/statistical_testing/internals/MultiKernelMMD_unittest.cc +++ b/tests/unit/statistical_testing/internals/MultiKernelMMD_unittest.cc @@ -87,7 +87,8 @@ TEST(MultiKernelMMD, biased_full) for (auto i=0, sigma=-5; icompute_joint_distance(distance)); + auto precomputed_distance=test->compute_joint_distance(distance); + kernel_mgr.set_precomputed_distance(precomputed_distance); SG_UNREF(distance); ComputeMMD tester; @@ -96,6 +97,7 @@ TEST(MultiKernelMMD, biased_full) tester.m_stype=stype; SGVector values=tester(kernel_mgr); kernel_mgr.unset_precomputed_distance(); + SG_UNREF(precomputed_distance); auto data_p=static_cast*>(feats_p)->get_feature_matrix(); auto data_q=static_cast*>(feats_q)->get_feature_matrix(); @@ -153,7 +155,8 @@ TEST(MultiKernelMMD, unbiased_full) for (auto i=0, sigma=-5; icompute_joint_distance(distance)); + auto precomputed_distance=test->compute_joint_distance(distance); + kernel_mgr.set_precomputed_distance(precomputed_distance); SG_UNREF(distance); ComputeMMD tester; @@ -162,6 +165,7 @@ TEST(MultiKernelMMD, unbiased_full) tester.m_stype=stype; SGVector values=tester(kernel_mgr); kernel_mgr.unset_precomputed_distance(); + SG_UNREF(precomputed_distance); auto data_p=static_cast*>(feats_p)->get_feature_matrix(); auto data_q=static_cast*>(feats_q)->get_feature_matrix(); @@ -219,7 +223,8 @@ TEST(MultiKernelMMD, unbiased_incomplete) for (auto i=0, sigma=-5; icompute_joint_distance(distance)); + auto precomputed_distance=test->compute_joint_distance(distance); + kernel_mgr.set_precomputed_distance(precomputed_distance); SG_UNREF(distance); ComputeMMD tester; @@ -228,6 +233,7 @@ TEST(MultiKernelMMD, unbiased_incomplete) tester.m_stype=stype; SGVector values=tester(kernel_mgr); kernel_mgr.unset_precomputed_distance(); + SG_UNREF(precomputed_distance); auto data_p=static_cast*>(feats_p)->get_feature_matrix(); auto data_q=static_cast*>(feats_q)->get_feature_matrix(); From cc702ee184fc6d02569dc4117acac62e12fcf98e Mon Sep 17 00:00:00 2001 From: Heiko Strathmann Date: Wed, 22 Feb 2017 16:18:57 +0000 Subject: [PATCH 13/24] make silent underflow failure explicit at runtime --- src/shogun/statistical_testing/internals/KernelManager.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/shogun/statistical_testing/internals/KernelManager.cpp b/src/shogun/statistical_testing/internals/KernelManager.cpp index af92ec25cff..cc2f4778ee2 100644 --- a/src/shogun/statistical_testing/internals/KernelManager.cpp +++ b/src/shogun/statistical_testing/internals/KernelManager.cpp @@ -104,6 +104,9 @@ void KernelManager::push_back(CKernel* kernel) const index_t KernelManager::num_kernels() const { + // TODO in case there is an underflow, at least it is not silent + // a better handling is to use index_t based Shogun data structures + ASSERT((index_t)m_kernels.size()>=0); return (index_t)m_kernels.size(); } From 269f5c982beea444826f2207a4319d44648d452c Mon Sep 17 00:00:00 2001 From: lambday Date: Thu, 23 Feb 2017 00:54:47 +0530 Subject: [PATCH 14/24] fixed memory leak in statistical hypothesis test unit tests --- .../kernelselection/internals/MaxCrossValidation.cpp | 5 ++++- .../internals/CrossValidationMMD_unittest.cc | 9 +++++++++ .../internals/DataFetcherFactory_unittest.cc | 2 ++ .../internals/PermutationMMD_unittest.cc | 6 ++++++ 4 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/shogun/statistical_testing/kernelselection/internals/MaxCrossValidation.cpp b/src/shogun/statistical_testing/kernelselection/internals/MaxCrossValidation.cpp index 5c8d78a7e60..550547c08a1 100644 --- a/src/shogun/statistical_testing/kernelselection/internals/MaxCrossValidation.cpp +++ b/src/shogun/statistical_testing/kernelselection/internals/MaxCrossValidation.cpp @@ -35,6 +35,7 @@ #include #include #include +#include #include #include #include @@ -106,10 +107,12 @@ void MaxCrossValidation::compute_measures() if (kernel_mgr.same_distance_type()) { CDistance* distance=kernel_mgr.get_distance_instance(); - kernel_mgr.set_precomputed_distance(estimator->compute_joint_distance(distance)); + auto precomputed_distance=estimator->compute_joint_distance(distance); + kernel_mgr.set_precomputed_distance(precomputed_distance); SG_UNREF(distance); compute(kernel_mgr); kernel_mgr.unset_precomputed_distance(); + SG_UNREF(precomputed_distance); } else { diff --git a/tests/unit/statistical_testing/internals/CrossValidationMMD_unittest.cc b/tests/unit/statistical_testing/internals/CrossValidationMMD_unittest.cc index 9e15ca824d2..0db570052db 100644 --- a/tests/unit/statistical_testing/internals/CrossValidationMMD_unittest.cc +++ b/tests/unit/statistical_testing/internals/CrossValidationMMD_unittest.cc @@ -138,6 +138,9 @@ TEST(CrossValidationMMD, biased_full) } } } + + SG_UNREF(feats_p); + SG_UNREF(feats_q); } TEST(CrossValidationMMD, unbiased_full) @@ -229,6 +232,9 @@ TEST(CrossValidationMMD, unbiased_full) } } } + + SG_UNREF(feats_p); + SG_UNREF(feats_q); } TEST(CrossValidationMMD, unbiased_incomplete) @@ -320,4 +326,7 @@ TEST(CrossValidationMMD, unbiased_incomplete) } } } + + SG_UNREF(feats_p); + SG_UNREF(feats_q); } diff --git a/tests/unit/statistical_testing/internals/DataFetcherFactory_unittest.cc b/tests/unit/statistical_testing/internals/DataFetcherFactory_unittest.cc index 36fd29a552e..db6ec989d90 100644 --- a/tests/unit/statistical_testing/internals/DataFetcherFactory_unittest.cc +++ b/tests/unit/statistical_testing/internals/DataFetcherFactory_unittest.cc @@ -61,4 +61,6 @@ TEST(DataFetcherFactory, get_instance) std::unique_ptr streaming_fetcher(DataFetcherFactory::get_instance(streaming_p)); ASSERT_TRUE(strcmp(streaming_fetcher->get_name(), "StreamingDataFetcher")==0); + + SG_UNREF(streaming_p); } diff --git a/tests/unit/statistical_testing/internals/PermutationMMD_unittest.cc b/tests/unit/statistical_testing/internals/PermutationMMD_unittest.cc index 4bd850d5522..778eb7cda38 100644 --- a/tests/unit/statistical_testing/internals/PermutationMMD_unittest.cc +++ b/tests/unit/statistical_testing/internals/PermutationMMD_unittest.cc @@ -361,6 +361,8 @@ TEST(PermutationMMD, biased_full_multi_kernel) auto feats_q=gen_q->get_streamed_features(m); auto merged_feats=static_cast*>(FeaturesUtil::create_merged_copy(feats_p, feats_q)); SG_REF(merged_feats); + SG_UNREF(feats_p); + SG_UNREF(feats_q); KernelManager kernel_mgr; for (auto i=0; iget_streamed_features(m); auto merged_feats=static_cast*>(FeaturesUtil::create_merged_copy(feats_p, feats_q)); SG_REF(merged_feats); + SG_UNREF(feats_p); + SG_UNREF(feats_q); KernelManager kernel_mgr; for (auto i=0; iget_streamed_features(m); auto merged_feats=static_cast*>(FeaturesUtil::create_merged_copy(feats_p, feats_q)); SG_REF(merged_feats); + SG_UNREF(feats_p); + SG_UNREF(feats_q); KernelManager kernel_mgr; for (auto i=0; i Date: Fri, 24 Feb 2017 01:02:20 +0530 Subject: [PATCH 15/24] replaced std::vector with SGVector --- src/shogun/statistical_testing/StreamingMMD.cpp | 5 +++-- .../internals/mmd/CrossValidationMMD.h | 11 +++++------ .../internals/mmd/PermutationMMD.h | 15 +++++++-------- 3 files changed, 15 insertions(+), 16 deletions(-) diff --git a/src/shogun/statistical_testing/StreamingMMD.cpp b/src/shogun/statistical_testing/StreamingMMD.cpp index 6b7c8a74b88..30abd6e50dd 100644 --- a/src/shogun/statistical_testing/StreamingMMD.cpp +++ b/src/shogun/statistical_testing/StreamingMMD.cpp @@ -264,7 +264,8 @@ std::pair, SGMatrix > CStreamingMMD::Self::comput std::fill(statistic.data(), statistic.data()+statistic.size(), 0); std::fill(Q.data(), Q.data()+Q.size(), 0); - std::vector term_counters_statistic(num_kernels, 1); + SGVector term_counters_statistic(num_kernels); + term_counters_statistic.set_const(1); SGMatrix term_counters_Q(num_kernels, num_kernels); std::fill(term_counters_Q.data(), term_counters_Q.data()+term_counters_Q.size(), 1); @@ -332,7 +333,7 @@ SGVector CStreamingMMD::Self::sample_null() REQUIRE(kernel != nullptr, "Kernel is not set!\n"); SGVector statistic(num_null_samples); - std::vector term_counters(num_null_samples); + SGVector term_counters(num_null_samples); std::fill(statistic.vector, statistic.vector+statistic.vlen, 0); std::fill(term_counters.data(), term_counters.data()+term_counters.size(), 1); diff --git a/src/shogun/statistical_testing/internals/mmd/CrossValidationMMD.h b/src/shogun/statistical_testing/internals/mmd/CrossValidationMMD.h index 4351d187276..b6bf5f52f07 100644 --- a/src/shogun/statistical_testing/internals/mmd/CrossValidationMMD.h +++ b/src/shogun/statistical_testing/internals/mmd/CrossValidationMMD.h @@ -111,14 +111,13 @@ struct CrossValidationMMD : PermutationMMD SGVector xy_wrapper(m_xy_inds.data(), m_xy_inds.size(), false); m_stack->add_subset(xy_wrapper); - m_permuted_inds.resize(m_xy_inds.size()); - SGVector permutation_wrapper(m_permuted_inds.data(), m_permuted_inds.size(), false); + m_permuted_inds.resize_vector(m_xy_inds.size()); for (auto n=0; nadd_subset(permutation_wrapper); + m_stack->add_subset(m_permuted_inds); SGVector inds=m_stack->get_last_subset()->get_subset_idx(); m_stack->remove_subset(); @@ -194,7 +193,7 @@ struct CrossValidationMMD : PermutationMMD unique_ptr m_kfold_y; unique_ptr m_stack; - std::vector m_xy_inds; + SGVector m_xy_inds; SGVector m_inverted_inds; SGMatrix m_rejections; @@ -227,7 +226,7 @@ struct CrossValidationMMD : PermutationMMD m_n_x=x_inds.size(); m_n_y=y_inds.size(); - m_xy_inds.resize(x_inds.size()+y_inds.size()); + m_xy_inds.resize_vector(x_inds.size()+y_inds.size()); std::copy(x_inds.data(), x_inds.data()+x_inds.size(), m_xy_inds.data()); std::copy(y_inds.data(), y_inds.data()+y_inds.size(), m_xy_inds.data()+x_inds.size()); } diff --git a/src/shogun/statistical_testing/internals/mmd/PermutationMMD.h b/src/shogun/statistical_testing/internals/mmd/PermutationMMD.h index 8f2315510d4..ccf44760c6f 100644 --- a/src/shogun/statistical_testing/internals/mmd/PermutationMMD.h +++ b/src/shogun/statistical_testing/internals/mmd/PermutationMMD.h @@ -197,17 +197,16 @@ struct PermutationMMD : ComputeMMD { ASSERT(m_num_null_samples>0); allocate_permutation_inds(); - SGVector sg_wrapper(m_permuted_inds.data(), m_permuted_inds.size(), false); for (auto n=0; n m_permuted_inds; + SGVector m_permuted_inds; std::vector > m_inverted_permuted_inds; SGMatrix m_all_inds; }; From 92a7362f7356854ad8fdafe8cecc1d831b1ac74a Mon Sep 17 00:00:00 2001 From: lambday Date: Sat, 25 Feb 2017 13:08:09 +0530 Subject: [PATCH 16/24] changed type of m_inverted_permuted_inds from vec> to SGMatrix --- .../internals/mmd/CrossValidationMMD.h | 23 ++++++------- .../internals/mmd/PermutationMMD.h | 34 ++++++++----------- 2 files changed, 26 insertions(+), 31 deletions(-) diff --git a/src/shogun/statistical_testing/internals/mmd/CrossValidationMMD.h b/src/shogun/statistical_testing/internals/mmd/CrossValidationMMD.h index b6bf5f52f07..3ab404db73f 100644 --- a/src/shogun/statistical_testing/internals/mmd/CrossValidationMMD.h +++ b/src/shogun/statistical_testing/internals/mmd/CrossValidationMMD.h @@ -105,13 +105,14 @@ struct CrossValidationMMD : PermutationMMD { generate_inds(current_fold); std::fill(m_inverted_inds.data(), m_inverted_inds.data()+m_inverted_inds.size(), -1); - for (size_t idx=0; idx xy_wrapper(m_xy_inds.data(), m_xy_inds.size(), false); m_stack->add_subset(xy_wrapper); m_permuted_inds.resize_vector(m_xy_inds.size()); + m_inverted_permuted_inds.set_const(-1); for (auto n=0; n inds=m_stack->get_last_subset()->get_subset_idx(); m_stack->remove_subset(); - std::fill(m_inverted_permuted_inds[n].data(), m_inverted_permuted_inds[n].data()+size, -1); for (int idx=0; idxremove_subset(); terms_t terms; for (auto i=0; i(size); - - m_inverted_permuted_inds.resize(m_num_null_samples); - for (auto i=0; i(size, m_num_null_samples); } void generate_inds(index_t current_fold) diff --git a/src/shogun/statistical_testing/internals/mmd/PermutationMMD.h b/src/shogun/statistical_testing/internals/mmd/PermutationMMD.h index ccf44760c6f..64aaffd3ecb 100644 --- a/src/shogun/statistical_testing/internals/mmd/PermutationMMD.h +++ b/src/shogun/statistical_testing/internals/mmd/PermutationMMD.h @@ -68,10 +68,10 @@ struct PermutationMMD : ComputeMMD terms_t terms; for (auto j=0; j=inverted_col) add_term_lower(terms, kernel(i, j), inverted_row, inverted_col); @@ -113,11 +113,12 @@ struct PermutationMMD : ComputeMMD terms_t null_terms; for (auto i=0; i(size, m_num_null_samples); - for (auto i=0; i(size, m_num_null_samples); } index_t m_num_null_samples; bool m_save_inds; SGVector m_permuted_inds; - std::vector > m_inverted_permuted_inds; + SGMatrix m_inverted_permuted_inds; SGMatrix m_all_inds; }; #endif // DOXYGEN_SHOULD_SKIP_THIS From 6db6482a3d8c18fd4675fde0db01fbfae972a7ca Mon Sep 17 00:00:00 2001 From: lambday Date: Sat, 25 Feb 2017 20:18:05 +0530 Subject: [PATCH 17/24] removed resize_vector and fixed realloc size bug --- .../internals/mmd/CrossValidationMMD.h | 20 ++++++++++++------- .../internals/mmd/PermutationMMD.h | 6 +++--- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/shogun/statistical_testing/internals/mmd/CrossValidationMMD.h b/src/shogun/statistical_testing/internals/mmd/CrossValidationMMD.h index 3ab404db73f..ca7dc40c3b2 100644 --- a/src/shogun/statistical_testing/internals/mmd/CrossValidationMMD.h +++ b/src/shogun/statistical_testing/internals/mmd/CrossValidationMMD.h @@ -108,11 +108,13 @@ struct CrossValidationMMD : PermutationMMD for (index_t idx=0; idx xy_wrapper(m_xy_inds.data(), m_xy_inds.size(), false); - m_stack->add_subset(xy_wrapper); + m_stack->add_subset(m_xy_inds); + + if (m_permuted_inds.size()!=m_xy_inds.size()) + m_permuted_inds=SGVector(m_xy_inds.size()); - m_permuted_inds.resize_vector(m_xy_inds.size()); m_inverted_permuted_inds.set_const(-1); + for (auto n=0; n x_inds=m_kfold_x->generate_subset_inverse(current_fold); SGVector y_inds=m_kfold_y->generate_subset_inverse(current_fold); - std::for_each(y_inds.data(), y_inds.data()+y_inds.size(), [this](index_t& val) { val += m_n_x; }); m_n_x=x_inds.size(); m_n_y=y_inds.size(); + auto size=m_n_x+m_n_y; + + if (m_xy_inds.size()!=size) + m_xy_inds=SGVector(size); + + std::copy(x_inds.data(), x_inds.data()+m_n_x, m_xy_inds.data()); - m_xy_inds.resize_vector(x_inds.size()+y_inds.size()); - std::copy(x_inds.data(), x_inds.data()+x_inds.size(), m_xy_inds.data()); - std::copy(y_inds.data(), y_inds.data()+y_inds.size(), m_xy_inds.data()+x_inds.size()); + std::for_each(y_inds.data(), y_inds.data()+m_n_y, [this](index_t& val) { val += m_n_x; }); + std::copy(y_inds.data(), y_inds.data()+y_inds.size(), m_xy_inds.data()+m_n_x); } }; #endif // DOXYGEN_SHOULD_SKIP_THIS diff --git a/src/shogun/statistical_testing/internals/mmd/PermutationMMD.h b/src/shogun/statistical_testing/internals/mmd/PermutationMMD.h index 64aaffd3ecb..e9161966996 100644 --- a/src/shogun/statistical_testing/internals/mmd/PermutationMMD.h +++ b/src/shogun/statistical_testing/internals/mmd/PermutationMMD.h @@ -224,12 +224,12 @@ struct PermutationMMD : ComputeMMD { const index_t size=m_n_x+m_n_y; if (m_permuted_inds.size()!=size) - m_permuted_inds.resize_vector(size); + m_permuted_inds=SGVector(size); - if (m_inverted_permuted_inds.num_cols!=m_num_null_samples && m_inverted_permuted_inds.num_rows!=size) + if (m_inverted_permuted_inds.num_cols!=m_num_null_samples || m_inverted_permuted_inds.num_rows!=size) m_inverted_permuted_inds=SGMatrix(size, m_num_null_samples); - if (m_save_inds && m_all_inds.num_cols!=m_num_null_samples && m_all_inds.num_rows!=size) + if (m_save_inds && (m_all_inds.num_cols!=m_num_null_samples || m_all_inds.num_rows!=size)) m_all_inds=SGMatrix(size, m_num_null_samples); } From 30940425ae0c7d8292c720b65589860559b39d29 Mon Sep 17 00:00:00 2001 From: lambday Date: Sat, 25 Feb 2017 21:36:26 +0530 Subject: [PATCH 18/24] fix cross-validation bug --- .../internals/mmd/CrossValidationMMD.h | 43 +++++++++---------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/src/shogun/statistical_testing/internals/mmd/CrossValidationMMD.h b/src/shogun/statistical_testing/internals/mmd/CrossValidationMMD.h index ca7dc40c3b2..df196e96210 100644 --- a/src/shogun/statistical_testing/internals/mmd/CrossValidationMMD.h +++ b/src/shogun/statistical_testing/internals/mmd/CrossValidationMMD.h @@ -186,21 +186,6 @@ struct CrossValidationMMD : PermutationMMD } } - index_t m_num_runs; - index_t m_num_folds; - static constexpr index_t DEFAULT_NUM_RUNS=10; - - float64_t m_alpha; - static constexpr float64_t DEFAULT_ALPHA=0.05; - - unique_ptr m_kfold_x; - unique_ptr m_kfold_y; - unique_ptr m_stack; - - SGVector m_xy_inds; - SGVector m_inverted_inds; - SGMatrix m_rejections; - void init() { SGVector dummy_labels_x(m_n_x); @@ -222,19 +207,33 @@ struct CrossValidationMMD : PermutationMMD { SGVector x_inds=m_kfold_x->generate_subset_inverse(current_fold); SGVector y_inds=m_kfold_y->generate_subset_inverse(current_fold); + std::for_each(y_inds.data(), y_inds.data()+y_inds.size(), [this](index_t& val) { val += m_n_x; }); m_n_x=x_inds.size(); m_n_y=y_inds.size(); - auto size=m_n_x+m_n_y; - - if (m_xy_inds.size()!=size) - m_xy_inds=SGVector(size); - std::copy(x_inds.data(), x_inds.data()+m_n_x, m_xy_inds.data()); + if (m_xy_inds.size()!=m_n_x+m_n_y) + m_xy_inds=SGVector(m_n_x+m_n_y); - std::for_each(y_inds.data(), y_inds.data()+m_n_y, [this](index_t& val) { val += m_n_x; }); - std::copy(y_inds.data(), y_inds.data()+y_inds.size(), m_xy_inds.data()+m_n_x); + std::copy(x_inds.data(), x_inds.data()+x_inds.size(), m_xy_inds.data()); + std::copy(y_inds.data(), y_inds.data()+y_inds.size(), m_xy_inds.data()+x_inds.size()); } + + index_t m_num_runs; + index_t m_num_folds; + static constexpr index_t DEFAULT_NUM_RUNS=10; + + float64_t m_alpha; + static constexpr float64_t DEFAULT_ALPHA=0.05; + + unique_ptr m_kfold_x; + unique_ptr m_kfold_y; + unique_ptr m_stack; + + SGVector m_xy_inds; + SGVector m_inverted_inds; + SGMatrix m_rejections; + }; #endif // DOXYGEN_SHOULD_SKIP_THIS } From 4c9fa38b18f683e4f65d2550736c3a7de12ca18c Mon Sep 17 00:00:00 2001 From: Viktor Gal Date: Wed, 22 Feb 2017 07:34:53 +0800 Subject: [PATCH 19/24] upgrade debian packaging and fix ENABLE_ cmake parameter --- cmake/ShogunUtils.cmake | 6 +++--- debian | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cmake/ShogunUtils.cmake b/cmake/ShogunUtils.cmake index 543d148ec2f..abc30353869 100644 --- a/cmake/ShogunUtils.cmake +++ b/cmake/ShogunUtils.cmake @@ -127,13 +127,13 @@ macro(ADD_LIBRARY_DEPENDENCY) set(multiValueArgs TARGETS) cmake_parse_arguments(ADD_LIBRARY_DEPENDENCY "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN}) STRING(TOUPPER ${ADD_LIBRARY_DEPENDENCY_LIBRARY} LIBRARY_PREFIX) - OPTION(ENABLE_${LIBRARY_PREFIX} "Use ${LIBRARY_PREFIX}" OFF) - if (${ADD_LIBRARY_DEPENDENCY_REQUIRED} OR ${ENABLE_${LIBRARY_PREFIX}}) + OPTION(ENABLE_${LIBRARY_PREFIX} "Use ${LIBRARY_PREFIX}" ON) + if (${ADD_LIBRARY_DEPENDENCY_REQUIRED}) find_package(${ADD_LIBRARY_DEPENDENCY_LIBRARY} REQUIRED ${ADD_LIBRARY_DEPENDENCY_VERSION}) else() find_package(${ADD_LIBRARY_DEPENDENCY_LIBRARY} ${ADD_LIBRARY_DEPENDENCY_VERSION}) endif() - if (${LIBRARY_PREFIX}_FOUND) + if (${LIBRARY_PREFIX}_FOUND AND ENABLE_${LIBRARY_PREFIX}) if (${LIBRARY_PREFIX}_INCLUDE_DIR) set(LIBRARY_HEADER ${${LIBRARY_PREFIX}_INCLUDE_DIR}) elseif (${LIBRARY_PREFIX}_INCLUDE_DIRS) diff --git a/debian b/debian index 1430d223d9a..add2d5b2eeb 160000 --- a/debian +++ b/debian @@ -1 +1 @@ -Subproject commit 1430d223d9a41dc270b5e6efe3a792dd6aa9a14e +Subproject commit add2d5b2eebb7631023cb26a3ff5614c396befbe From 8f9bae313aa56e8dabc64a7ba48c2a226b2ea252 Mon Sep 17 00:00:00 2001 From: Viktor Gal Date: Mon, 27 Feb 2017 08:29:38 +0800 Subject: [PATCH 20/24] Disable duplicate functionality in StreamingFeatures currently it is unclear what should be the functionality of duplication of a stream, hence desiabling it. --- .../features/streaming/StreamingDenseFeatures.cpp | 5 ----- .../features/streaming/StreamingDenseFeatures.h | 7 ------- src/shogun/features/streaming/StreamingFeatures.h | 13 +++++++++++++ .../streaming/StreamingHashedDenseFeatures.cpp | 6 ------ .../streaming/StreamingHashedDenseFeatures.h | 7 ------- .../streaming/StreamingHashedDocDotFeatures.cpp | 5 ----- .../streaming/StreamingHashedDocDotFeatures.h | 7 ------- .../streaming/StreamingHashedSparseFeatures.cpp | 6 ------ .../streaming/StreamingHashedSparseFeatures.h | 7 ------- .../features/streaming/StreamingSparseFeatures.cpp | 6 ------ .../features/streaming/StreamingSparseFeatures.h | 7 ------- .../features/streaming/StreamingStringFeatures.cpp | 6 ------ .../features/streaming/StreamingStringFeatures.h | 7 ------- .../features/streaming/StreamingVwFeatures.cpp | 5 ----- src/shogun/features/streaming/StreamingVwFeatures.h | 7 ------- 15 files changed, 13 insertions(+), 88 deletions(-) diff --git a/src/shogun/features/streaming/StreamingDenseFeatures.cpp b/src/shogun/features/streaming/StreamingDenseFeatures.cpp index 279dece6a55..6aeeab73a7f 100644 --- a/src/shogun/features/streaming/StreamingDenseFeatures.cpp +++ b/src/shogun/features/streaming/StreamingDenseFeatures.cpp @@ -140,11 +140,6 @@ template int32_t CStreamingDenseFeatures::get_nnz_features_for_vecto return current_vector.vlen; } -template CFeatures* CStreamingDenseFeatures::duplicate() const -{ - return new CStreamingDenseFeatures(*this); -} - template int32_t CStreamingDenseFeatures::get_num_vectors() const { return 1; diff --git a/src/shogun/features/streaming/StreamingDenseFeatures.h b/src/shogun/features/streaming/StreamingDenseFeatures.h index a207605deb4..2342fc2b7bb 100644 --- a/src/shogun/features/streaming/StreamingDenseFeatures.h +++ b/src/shogun/features/streaming/StreamingDenseFeatures.h @@ -244,13 +244,6 @@ template class CStreamingDenseFeatures: */ virtual EFeatureClass get_feature_class() const; - /** - * Duplicate the object. - * - * @return a duplicate object as CFeatures* - */ - virtual CFeatures* duplicate() const; - /** * Return the name. * diff --git a/src/shogun/features/streaming/StreamingFeatures.h b/src/shogun/features/streaming/StreamingFeatures.h index 1314e785d28..541ed1b8451 100644 --- a/src/shogun/features/streaming/StreamingFeatures.h +++ b/src/shogun/features/streaming/StreamingFeatures.h @@ -80,6 +80,8 @@ class CStreamingFeatures : public CFeatures */ CStreamingFeatures(); + CStreamingFeatures(const CStreamingFeatures&) = delete; + /** * Destructor */ @@ -195,6 +197,17 @@ class CStreamingFeatures : public CFeatures return NULL; } + /** + * Duplicate the object. + * + * @return a duplicate object as CFeatures* + */ + virtual CFeatures* duplicate() const + { + SG_NOTIMPLEMENTED + return NULL; + } + protected: /// Whether examples are labelled or not. diff --git a/src/shogun/features/streaming/StreamingHashedDenseFeatures.cpp b/src/shogun/features/streaming/StreamingHashedDenseFeatures.cpp index 1e6890e8543..bbaa66e6432 100644 --- a/src/shogun/features/streaming/StreamingHashedDenseFeatures.cpp +++ b/src/shogun/features/streaming/StreamingHashedDenseFeatures.cpp @@ -134,12 +134,6 @@ int32_t CStreamingHashedDenseFeatures::get_num_vectors() const return 1; } -template -CFeatures* CStreamingHashedDenseFeatures::duplicate() const -{ - return new CStreamingHashedDenseFeatures(*this); -} - template void CStreamingHashedDenseFeatures::set_vector_reader() { diff --git a/src/shogun/features/streaming/StreamingHashedDenseFeatures.h b/src/shogun/features/streaming/StreamingHashedDenseFeatures.h index 37fd751738c..38b08aa7471 100644 --- a/src/shogun/features/streaming/StreamingHashedDenseFeatures.h +++ b/src/shogun/features/streaming/StreamingHashedDenseFeatures.h @@ -117,13 +117,6 @@ template class CStreamingHashedDenseFeatures : public CStreamingDotFe */ virtual int32_t get_num_vectors() const; - /** - * Duplicate the object. - * - * @return a duplicate object as CFeatures* - */ - virtual CFeatures* duplicate() const; - /** * Sets the read function (in case the examples are * unlabelled) to get_*_vector() from CStreamingFile. diff --git a/src/shogun/features/streaming/StreamingHashedDocDotFeatures.cpp b/src/shogun/features/streaming/StreamingHashedDocDotFeatures.cpp index 9fa189df3b7..41dcd5744f4 100644 --- a/src/shogun/features/streaming/StreamingHashedDocDotFeatures.cpp +++ b/src/shogun/features/streaming/StreamingHashedDocDotFeatures.cpp @@ -124,11 +124,6 @@ const char* CStreamingHashedDocDotFeatures::get_name() const return "StreamingHashedDocDotFeatures"; } -CFeatures* CStreamingHashedDocDotFeatures::duplicate() const -{ - return new CStreamingHashedDocDotFeatures(*this); -} - EFeatureType CStreamingHashedDocDotFeatures::get_feature_type() const { return F_UINT; diff --git a/src/shogun/features/streaming/StreamingHashedDocDotFeatures.h b/src/shogun/features/streaming/StreamingHashedDocDotFeatures.h index e19fd652ca6..f0c0ee9a9e0 100644 --- a/src/shogun/features/streaming/StreamingHashedDocDotFeatures.h +++ b/src/shogun/features/streaming/StreamingHashedDocDotFeatures.h @@ -129,13 +129,6 @@ class CStreamingHashedDocDotFeatures : public CStreamingDotFeatures */ virtual int32_t get_num_vectors() const; - /** - * Duplicate the object. - * - * @return a duplicate object as CFeatures* - */ - virtual CFeatures* duplicate() const; - /** * Sets the read function (in case the examples are * unlabelled) to get_*_vector() from CStreamingFile. diff --git a/src/shogun/features/streaming/StreamingHashedSparseFeatures.cpp b/src/shogun/features/streaming/StreamingHashedSparseFeatures.cpp index 4837db7d008..063d11eee54 100644 --- a/src/shogun/features/streaming/StreamingHashedSparseFeatures.cpp +++ b/src/shogun/features/streaming/StreamingHashedSparseFeatures.cpp @@ -136,12 +136,6 @@ int32_t CStreamingHashedSparseFeatures::get_num_vectors() const return 1; } -template -CFeatures* CStreamingHashedSparseFeatures::duplicate() const -{ - return new CStreamingHashedSparseFeatures(*this); -} - template void CStreamingHashedSparseFeatures::set_vector_reader() { diff --git a/src/shogun/features/streaming/StreamingHashedSparseFeatures.h b/src/shogun/features/streaming/StreamingHashedSparseFeatures.h index 4ec7926baa3..6bf67ac9640 100644 --- a/src/shogun/features/streaming/StreamingHashedSparseFeatures.h +++ b/src/shogun/features/streaming/StreamingHashedSparseFeatures.h @@ -117,13 +117,6 @@ template class CStreamingHashedSparseFeatures : public CStreamingDotF */ virtual int32_t get_num_vectors() const; - /** - * Duplicate the object. - * - * @return a duplicate object as CFeatures* - */ - virtual CFeatures* duplicate() const; - /** * Sets the read function (in case the examples are * unlabelled) to get_*_vector() from CStreamingFile. diff --git a/src/shogun/features/streaming/StreamingSparseFeatures.cpp b/src/shogun/features/streaming/StreamingSparseFeatures.cpp index 20aedbaa76f..9c43aa78804 100644 --- a/src/shogun/features/streaming/StreamingSparseFeatures.cpp +++ b/src/shogun/features/streaming/StreamingSparseFeatures.cpp @@ -219,12 +219,6 @@ void CStreamingSparseFeatures::sort_features() ASSERT(old_ptr == current_sgvector.features); } -template -CFeatures* CStreamingSparseFeatures::duplicate() const -{ - return new CStreamingSparseFeatures(*this); -} - template int32_t CStreamingSparseFeatures::get_num_vectors() const { diff --git a/src/shogun/features/streaming/StreamingSparseFeatures.h b/src/shogun/features/streaming/StreamingSparseFeatures.h index 42a5f15f7ee..68dcc503ee3 100644 --- a/src/shogun/features/streaming/StreamingSparseFeatures.h +++ b/src/shogun/features/streaming/StreamingSparseFeatures.h @@ -309,13 +309,6 @@ template class CStreamingSparseFeatures : public CStreamingDotFeatures */ virtual EFeatureClass get_feature_class() const; - /** - * Duplicate the object. - * - * @return a duplicate object as CFeatures* - */ - virtual CFeatures* duplicate() const; - /** * Return the name. * diff --git a/src/shogun/features/streaming/StreamingStringFeatures.cpp b/src/shogun/features/streaming/StreamingStringFeatures.cpp index fbac64687a0..84a7d86e949 100644 --- a/src/shogun/features/streaming/StreamingStringFeatures.cpp +++ b/src/shogun/features/streaming/StreamingStringFeatures.cpp @@ -80,12 +80,6 @@ floatmax_t CStreamingStringFeatures::get_num_symbols() return num_symbols; } -template -CFeatures* CStreamingStringFeatures::duplicate() const -{ - return new CStreamingStringFeatures(*this); -} - template int32_t CStreamingStringFeatures::get_num_vectors() const { diff --git a/src/shogun/features/streaming/StreamingStringFeatures.h b/src/shogun/features/streaming/StreamingStringFeatures.h index b317811259c..c577b91d42c 100644 --- a/src/shogun/features/streaming/StreamingStringFeatures.h +++ b/src/shogun/features/streaming/StreamingStringFeatures.h @@ -198,13 +198,6 @@ template class CStreamingStringFeatures : public CStreamingFeatures */ virtual EFeatureClass get_feature_class() const; - /** - * Duplicate the object. - * - * @return a duplicate object as CFeatures* - */ - virtual CFeatures* duplicate() const; - /** * Return the name. * diff --git a/src/shogun/features/streaming/StreamingVwFeatures.cpp b/src/shogun/features/streaming/StreamingVwFeatures.cpp index 9c00f9e1a23..a67e190262d 100644 --- a/src/shogun/features/streaming/StreamingVwFeatures.cpp +++ b/src/shogun/features/streaming/StreamingVwFeatures.cpp @@ -46,11 +46,6 @@ CStreamingVwFeatures::~CStreamingVwFeatures() SG_UNREF(env); } -CFeatures* CStreamingVwFeatures::duplicate() const -{ - return new CStreamingVwFeatures(*this); -} - void CStreamingVwFeatures::set_vector_reader() { parser.set_read_vector(&CStreamingFile::get_vector); diff --git a/src/shogun/features/streaming/StreamingVwFeatures.h b/src/shogun/features/streaming/StreamingVwFeatures.h index fd79a22b5d1..b56a3d15d24 100644 --- a/src/shogun/features/streaming/StreamingVwFeatures.h +++ b/src/shogun/features/streaming/StreamingVwFeatures.h @@ -80,13 +80,6 @@ class CStreamingVwFeatures : public CStreamingDotFeatures */ ~CStreamingVwFeatures(); - /** - * Duplicate this object - * - * @return a copy of this object - */ - CFeatures* duplicate() const; - /** * Sets the read function (in case the examples are * unlabelled) to get_*_vector() from CStreamingFile. From 426064ad787d3d5d71fc6da8ac6f5f5fca4fa2d9 Mon Sep 17 00:00:00 2001 From: Viktor Gal Date: Mon, 27 Feb 2017 08:30:22 +0800 Subject: [PATCH 21/24] lower unit test precision in CustomKernel --- tests/unit/kernel/CustomKernel_unittest.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/kernel/CustomKernel_unittest.cc b/tests/unit/kernel/CustomKernel_unittest.cc index b3d7d0c582f..24405caf01c 100644 --- a/tests/unit/kernel/CustomKernel_unittest.cc +++ b/tests/unit/kernel/CustomKernel_unittest.cc @@ -243,13 +243,13 @@ TEST(CustomKernelTest, sum_symmetric_block) sum1=kernel->sum_symmetric_block(0, m+n); sum2=precomputed_kernel->sum_symmetric_block(0, m+n); - EXPECT_NEAR(sum1, sum2, 1E-4); + EXPECT_NEAR(sum1, sum2, 1E-3); // overall - sum(K(X.Y, X'.Y')) (with diag) sum1=kernel->sum_symmetric_block(0, m+n, false); sum2=precomputed_kernel->sum_symmetric_block(0, m+n, false); - EXPECT_NEAR(sum1, sum2, 1E-4); + EXPECT_NEAR(sum1, sum2, 1E-3); SG_UNREF(precomputed_kernel); SG_UNREF(kernel); From 966c37305aab39651f58da94d8d9fda3eb27efda Mon Sep 17 00:00:00 2001 From: Viktor Gal Date: Tue, 28 Feb 2017 13:15:21 +0800 Subject: [PATCH 22/24] Add terminate atomic flag for parsing thread in InputParser this allows graceful stopping of the parsing thread Remove pthread implementation from InputParser finetune the memory ordering of the atomic counter in RefCount --- src/shogun/io/streaming/InputParser.h | 140 +++++--------------------- src/shogun/lib/RefCount.cpp | 30 +----- src/shogun/lib/RefCount.h | 24 ++--- 3 files changed, 35 insertions(+), 159 deletions(-) diff --git a/src/shogun/io/streaming/InputParser.h b/src/shogun/io/streaming/InputParser.h index 652d0db3a1d..c4000f1a2b2 100644 --- a/src/shogun/io/streaming/InputParser.h +++ b/src/shogun/io/streaming/InputParser.h @@ -13,20 +13,14 @@ #include -#if defined(HAVE_CXX11) || defined(HAVE_PTHREAD) - #include #include #include #include -#ifdef HAVE_CXX11 #include #include #include #include -#elif defined(HAVE_PTHREAD) -#include -#endif #define PARSER_DEFAULT_BUFFSIZE 100 @@ -324,11 +318,7 @@ template class CInputParser CStreamingFile* input_source; /// Thread in which the parser runs -#ifdef HAVE_CXX11 - std::shared_ptr parse_thread; -#elif defined(HAVE_PTHREAD) - pthread_t parse_thread; -#endif + std::thread parse_thread; /// The ring of examples, stored as they are parsed CParseBuffer* examples_ring; @@ -361,18 +351,13 @@ template class CInputParser int32_t ring_size; /// Mutex which is used when getting/setting state of examples (whether a new example is ready) -#ifdef HAVE_CXX11 - std::shared_ptr examples_state_lock; -#elif defined(HAVE_PTHREAD) - pthread_mutex_t examples_state_lock; -#endif + std::mutex examples_state_lock; /// Condition variable to indicate change of state of examples -#ifdef HAVE_CXX11 - std::shared_ptr examples_state_changed; -#elif defined(HAVE_PTHREAD) - pthread_cond_t examples_state_changed; -#endif + std::condition_variable examples_state_changed; + + /// Flag that indicate that the parsing thread should continue reading + std::atomic keep_running; }; @@ -393,28 +378,15 @@ template template CInputParser::CInputParser() { - /* this line was commented out when I found it. However, the mutex locks - * have to be initialised. Otherwise uninitialised memory error */ - //init(NULL, true, PARSER_DEFAULT_BUFFSIZE); -#if HAVE_CXX11 - examples_state_lock = std::make_shared(); - examples_state_changed = std::make_shared(); -#elif defined(HAVE_PTHREAD) - pthread_mutex_init(&examples_state_lock, NULL); - pthread_cond_init(&examples_state_changed, NULL); -#endif - examples_ring=NULL; + examples_ring = nullptr; parsing_done=true; reading_done=true; + keep_running.store(false, std::memory_order_release); } template CInputParser::~CInputParser() { -#if !defined(HAVE_CXX11) && defined(HAVE_PTHREAD) - pthread_mutex_destroy(&examples_state_lock); - pthread_cond_destroy(&examples_state_changed); -#endif SG_UNREF(examples_ring); } @@ -469,11 +441,8 @@ template SG_SDEBUG("creating parse thread\n") if (examples_ring) examples_ring->init_vector(); -#ifdef HAVE_CXX11 - parse_thread.reset(new std::thread(&parse_loop_entry_point, this)); -#elif defined(HAVE_PTHREAD) - pthread_create(&parse_thread, NULL, parse_loop_entry_point, this); -#endif + keep_running.store(true, std::memory_order_release); + parse_thread = std::thread(&parse_loop_entry_point, this); SG_SDEBUG("leaving CInputParser::start_parser()\n") } @@ -491,11 +460,7 @@ template { SG_SDEBUG("entering CInputParser::is_running()\n") bool ret; -#ifdef HAVE_CXX11 - std::lock_guard lock(*examples_state_lock); -#elif defined(HAVE_PTHREAD) - pthread_mutex_lock(&examples_state_lock); -#endif + std::lock_guard lock(examples_state_lock); if (parsing_done) if (reading_done) @@ -504,9 +469,6 @@ template ret = true; else ret = false; -#if !defined(HAVE_CXX11) && defined(HAVE_PTHREAD) - pthread_mutex_unlock(&examples_state_lock); -#endif SG_SDEBUG("leaving CInputParser::is_running(), returning %d\n", ret) return ret; @@ -556,26 +518,15 @@ template void* CInputParser::main_parse_loop(void* params) CInputParser* this_obj = (CInputParser *) params; this->input_source = this_obj->input_source; - while (1) + while (keep_running.load(std::memory_order_acquire)) { -#ifdef HAVE_CXX11 - std::unique_lock lock(*examples_state_lock); -#elif defined(HAVE_PTHREAD) - pthread_mutex_lock(&examples_state_lock); -#endif + std::unique_lock lock(examples_state_lock); + if (parsing_done) { -#if !defined(HAVE_CXX11) && defined(HAVE_PTHREAD) - pthread_mutex_unlock(&examples_state_lock); -#endif return NULL; } -#ifdef HAVE_CXX11 lock.unlock(); -#elif defined(HAVE_PTHREAD) - pthread_mutex_unlock(&examples_state_lock); - pthread_testcancel(); -#endif current_example = examples_ring->get_free_example(); current_feature_vector = current_example->fv; @@ -589,16 +540,9 @@ template void* CInputParser::main_parse_loop(void* params) if (current_len < 0) { -#ifdef HAVE_CXX11 lock.lock(); parsing_done = true; - examples_state_changed->notify_one(); -#elif defined(HAVE_PTHREAD) - pthread_mutex_lock(&examples_state_lock); - parsing_done = true; - pthread_cond_signal(&examples_state_changed); - pthread_mutex_unlock(&examples_state_lock); -#endif + examples_state_changed.notify_one(); return NULL; } @@ -607,16 +551,9 @@ template void* CInputParser::main_parse_loop(void* params) current_example->length = current_len; examples_ring->copy_example(current_example); -#ifdef HAVE_CXX11 lock.lock(); number_of_vectors_parsed++; - examples_state_changed->notify_one(); -#elif defined(HAVE_PTHREAD) - pthread_mutex_lock(&examples_state_lock); - number_of_vectors_parsed++; - pthread_cond_signal(&examples_state_changed); - pthread_mutex_unlock(&examples_state_lock); -#endif + examples_state_changed.notify_one(); } return NULL; } @@ -632,11 +569,7 @@ template Example* CInputParser::retrieve_example() { reading_done = true; /* Signal to waiting threads that no more examples are left */ -#ifdef HAVE_CXX11 - examples_state_changed->notify_one(); -#elif defined(HAVE_PTHREAD) - pthread_cond_signal(&examples_state_changed); -#endif + examples_state_changed.notify_one(); return NULL; } } @@ -665,16 +598,12 @@ template int32_t CInputParser::get_next_example(T* &fv, Example *ex; - while (1) + while (keep_running.load(std::memory_order_acquire)) { if (reading_done) return 0; -#ifdef HAVE_CXX11 - std::unique_lock lock(*examples_state_lock); -#elif defined(HAVE_PTHREAD) - pthread_mutex_lock(&examples_state_lock); -#endif + std::unique_lock lock(examples_state_lock); ex = retrieve_example(); if (ex == NULL) @@ -682,29 +611,18 @@ template int32_t CInputParser::get_next_example(T* &fv, if (reading_done) { /* No more examples left, return */ -#if !defined(HAVE_CXX11) && defined(HAVE_PTHREAD) - pthread_mutex_unlock(&examples_state_lock); -#endif return 0; } else { /* Examples left, wait for one to become ready */ -#ifdef HAVE_CXX11 - examples_state_changed->wait(lock); -#elif defined(HAVE_PTHREAD) - pthread_cond_wait(&examples_state_changed, &examples_state_lock); - pthread_mutex_unlock(&examples_state_lock); -#endif + examples_state_changed.wait(lock); continue; } } else { /* Example ready, return the example */ -#if !defined(HAVE_CXX11) && defined(HAVE_PTHREAD) - pthread_mutex_unlock(&examples_state_lock); -#endif break; } } @@ -734,25 +652,19 @@ template void CInputParser::end_parser() { SG_SDEBUG("entering CInputParser::end_parser\n") SG_SDEBUG("joining parse thread\n") -#ifdef HAVE_CXX11 - parse_thread->join(); -#elif defined(HAVE_PTHREAD) - pthread_join(parse_thread, NULL); -#endif + if (parse_thread.joinable()) + parse_thread.join(); SG_SDEBUG("leaving CInputParser::end_parser\n") } template void CInputParser::exit_parser() { SG_SDEBUG("cancelling parse thread\n") -#ifdef HAVE_CXX11 - parse_thread.reset(); -#elif defined(HAVE_PTHREAD) - pthread_cancel(parse_thread); -#endif + keep_running.store(false, std::memory_order_release); + examples_state_changed.notify_one(); + if (parse_thread.joinable()) + parse_thread.join(); } } -#endif /* defined(HAVE_CXX11) || defined(HAVE_PTHREAD) */ - #endif // __INPUTPARSER_H__ diff --git a/src/shogun/lib/RefCount.cpp b/src/shogun/lib/RefCount.cpp index e9517e28cbc..9c1ff8429c7 100644 --- a/src/shogun/lib/RefCount.cpp +++ b/src/shogun/lib/RefCount.cpp @@ -4,39 +4,15 @@ using namespace shogun; int32_t RefCount::ref() { -#ifdef HAVE_CXX11_ATOMIC - int32_t count = rc.fetch_add(1)+1; -#else - lock.lock(); - int32_t count = ++rc; - lock.unlock(); -#endif - - return count; + return rc.fetch_add(1, std::memory_order_relaxed)+1; } int32_t RefCount::unref() { -#ifdef HAVE_CXX11_ATOMIC - int32_t count = rc.fetch_sub(1)-1; -#else - lock.lock(); - int32_t count = --rc; - lock.unlock(); -#endif - - return count; + return rc.fetch_sub(1, std::memory_order_acquire)-1; } int32_t RefCount::ref_count() { -#ifdef HAVE_CXX11_ATOMIC - int32_t count = rc.load(); -#else - lock.lock(); - int32_t count = rc; - lock.unlock(); -#endif - - return count; + return rc.load(std::memory_order_acquire); } diff --git a/src/shogun/lib/RefCount.h b/src/shogun/lib/RefCount.h index 137d04d7c0b..86c0678ae04 100644 --- a/src/shogun/lib/RefCount.h +++ b/src/shogun/lib/RefCount.h @@ -1,15 +1,10 @@ -#include - -#ifdef HAVE_CXX11_ATOMIC -#include -#endif - -#include -#include - #ifndef _REFCOUNT__H__ #define _REFCOUNT__H__ +#include +#include +#include + namespace shogun { /** brief This class implements a thread-safe counter used for @@ -22,7 +17,7 @@ class RefCount * * @param ref_start starting value for counter */ - RefCount(int32_t ref_start=0) : rc(ref_start) {} + RefCount(int32_t ref_start=0) : rc(ref_start) {}; /** Increase ref count * @@ -43,14 +38,7 @@ class RefCount int32_t ref_count(); /** reference count */ -#ifdef HAVE_CXX11_ATOMIC - volatile std::atomic rc; -#else - int32_t rc; - - /** the lock */ - CLock lock; -#endif + std::atomic rc; }; } From 200b1d1e78a914cb92e42ab3f4003849fdf61515 Mon Sep 17 00:00:00 2001 From: Viktor Gal Date: Tue, 28 Feb 2017 14:28:07 +0800 Subject: [PATCH 23/24] Move Lock and RefCount to be a header only class add SG_FORCED_INLINE macro to force inline of a function add CPU_CACHE_LINE_SIZE (backported from linalg_refactor) --- src/shogun/io/streaming/InputParser.h | 2 +- src/shogun/lib/Lock.cpp | 77 --------------------------- src/shogun/lib/Lock.h | 31 +++++------ src/shogun/lib/RefCount.cpp | 18 ------- src/shogun/lib/RefCount.h | 16 ++++-- src/shogun/lib/common.h | 10 +++- src/shogun/lib/cpu.h | 45 ++++++++++++++++ 7 files changed, 84 insertions(+), 115 deletions(-) delete mode 100644 src/shogun/lib/Lock.cpp delete mode 100644 src/shogun/lib/RefCount.cpp create mode 100644 src/shogun/lib/cpu.h diff --git a/src/shogun/io/streaming/InputParser.h b/src/shogun/io/streaming/InputParser.h index c4000f1a2b2..5c34385f0a4 100644 --- a/src/shogun/io/streaming/InputParser.h +++ b/src/shogun/io/streaming/InputParser.h @@ -357,7 +357,7 @@ template class CInputParser std::condition_variable examples_state_changed; /// Flag that indicate that the parsing thread should continue reading - std::atomic keep_running; + alignas(CPU_CACHE_LINE_SIZE) std::atomic_bool keep_running; }; diff --git a/src/shogun/lib/Lock.cpp b/src/shogun/lib/Lock.cpp deleted file mode 100644 index e20b7e6ba5d..00000000000 --- a/src/shogun/lib/Lock.cpp +++ /dev/null @@ -1,77 +0,0 @@ -/* - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 3 of the License, or - * (at your option) any later version. - * - * Copyright (C) 2013 Soeren Sonnenburg - */ -#include -#include -#include - -#ifdef HAVE_PTHREAD -#include -#ifdef USE_SPINLOCKS -#ifdef DARWIN -#include - #define PTHREAD_LOCK_T OSSpinLock - #define PTHREAD_LOCK_INIT(lock) *lock = OS_SPINLOCK_INIT - #define PTHREAD_LOCK_DESTROY(lock) - #define PTHREAD_LOCK(lock) OSSpinLockLock(lock) - #define PTHREAD_UNLOCK(lock) OSSpinLockUnlock(lock) -#else - #define PTHREAD_LOCK_T pthread_spinlock_t - #define PTHREAD_LOCK_INIT(lock) pthread_spin_init(lock, 0) - #define PTHREAD_LOCK_DESTROY(lock) pthread_spin_destroy(lock) - #define PTHREAD_LOCK(lock) pthread_spin_lock(lock) - #define PTHREAD_UNLOCK(lock) pthread_spin_unlock(lock) -#endif // DARWIN -#else - #define PTHREAD_LOCK_T pthread_mutex_t - #define PTHREAD_LOCK_INIT(lock) pthread_mutex_init(lock, NULL) - #define PTHREAD_LOCK_DESTROY(lock) pthread_mutex_destroy(lock) - #define PTHREAD_LOCK(lock) pthread_mutex_lock(lock) - #define PTHREAD_UNLOCK(lock) pthread_mutex_unlock(lock) -#endif // USE_SPINLOCKS -#endif // HAVE_PTHREAD - -using namespace shogun; - -CLock::CLock() -{ -#if !defined(HAVE_CXX11_ATOMIC) && defined(HAVE_PTHREAD) - lock_object=(void*) SG_MALLOC(PTHREAD_LOCK_T, 1); - PTHREAD_LOCK_INIT((PTHREAD_LOCK_T*) lock_object); -#endif -} - -CLock::~CLock() -{ -#if !defined(HAVE_CXX11_ATOMIC) && defined(HAVE_PTHREAD) - PTHREAD_LOCK_DESTROY((PTHREAD_LOCK_T*) lock_object); - SG_FREE(lock_object); -#endif -} - -void CLock::lock() -{ -#ifdef HAVE_CXX11_ATOMIC - while(m_flag.test_and_set(std::memory_order_acquire)); -#elif HAVE_PTHREAD - PTHREAD_LOCK((PTHREAD_LOCK_T*) lock_object); -#else - SG_NOTIMPLEMENTED -#endif -} - -void CLock::unlock() -{ -#ifdef HAVE_CXX11_ATOMIC - m_flag.clear(std::memory_order_release); -#elif HAVE_PTHREAD - PTHREAD_UNLOCK((PTHREAD_LOCK_T*) lock_object); -#else - SG_NOTIMPLEMENTED -#endif -} diff --git a/src/shogun/lib/Lock.h b/src/shogun/lib/Lock.h index ea2df216126..3467b965c53 100644 --- a/src/shogun/lib/Lock.h +++ b/src/shogun/lib/Lock.h @@ -10,10 +10,8 @@ #define __LOCK_H__ #include - -#ifdef HAVE_CXX11_ATOMIC +#include #include -#endif namespace shogun { @@ -21,23 +19,26 @@ namespace shogun class CLock { public: - /** default constructor */ - CLock(); - /** de-structor */ - ~CLock(); - /** lock the object */ - void lock(); + SG_FORCED_INLINE void lock() + { + do + { + while (m_locked.load(std::memory_order_relaxed)) + CpuRelax(); + } + while (m_locked.exchange(true, std::memory_order_acquire)); + } + /** unlock the object (must be called as often as lock) */ - void unlock(); + SG_FORCED_INLINE void unlock() + { + m_locked.store(false, std::memory_order_release); + } private: /** lock object */ -#ifdef HAVE_CXX11_ATOMIC - std::atomic_flag m_flag = ATOMIC_FLAG_INIT; -#else - void* lock_object; -#endif + std::atomic_bool m_locked = { false }; }; } #endif // __LOCK_H__ diff --git a/src/shogun/lib/RefCount.cpp b/src/shogun/lib/RefCount.cpp deleted file mode 100644 index 9c1ff8429c7..00000000000 --- a/src/shogun/lib/RefCount.cpp +++ /dev/null @@ -1,18 +0,0 @@ -#include - -using namespace shogun; - -int32_t RefCount::ref() -{ - return rc.fetch_add(1, std::memory_order_relaxed)+1; -} - -int32_t RefCount::unref() -{ - return rc.fetch_sub(1, std::memory_order_acquire)-1; -} - -int32_t RefCount::ref_count() -{ - return rc.load(std::memory_order_acquire); -} diff --git a/src/shogun/lib/RefCount.h b/src/shogun/lib/RefCount.h index 86c0678ae04..8bb4c0b13aa 100644 --- a/src/shogun/lib/RefCount.h +++ b/src/shogun/lib/RefCount.h @@ -23,20 +23,30 @@ class RefCount * * @return the new reference count */ - int32_t ref(); + SG_FORCED_INLINE int32_t ref() + { + return rc.fetch_add(1, std::memory_order_relaxed)+1; + } /** Decrease reference count * * @return the new reference count */ - int32_t unref(); + SG_FORCED_INLINE int32_t unref() + { + return rc.fetch_sub(1, std::memory_order_acquire)-1; + } /** Get the reference count * * @return the reference count */ - int32_t ref_count(); + SG_FORCED_INLINE int32_t ref_count() + { + return rc.load(std::memory_order_acquire); + } +private: /** reference count */ std::atomic rc; }; diff --git a/src/shogun/lib/common.h b/src/shogun/lib/common.h index 086ff3d9f4f..e8787f816ce 100644 --- a/src/shogun/lib/common.h +++ b/src/shogun/lib/common.h @@ -76,12 +76,20 @@ typedef int32_t index_t; typedef std::complex complex128_t; -#define CPU_CACHE_LINE_SIZE_BYTES 8 +/** CPU cache line size */ +constexpr size_t CPU_CACHE_LINE_SIZE = 64; +constexpr size_t CPU_CACHE_LINE_SIZE_BYTES = CPU_CACHE_LINE_SIZE/8; #ifdef _WIN32 #include typedef SSIZE_T ssize_t; #endif +#ifdef _MSC_VER +#define SG_FORCED_INLINE __forceinline +#else +#define SG_FORCED_INLINE inline __attribute__((always_inline)) +#endif + #include #endif //__COMMON_H__ diff --git a/src/shogun/lib/cpu.h b/src/shogun/lib/cpu.h new file mode 100644 index 00000000000..0916756d799 --- /dev/null +++ b/src/shogun/lib/cpu.h @@ -0,0 +1,45 @@ +/* + * Copyright (c) The Shogun Machine Learning Toolbox + * Written (w) 2017 - Viktor Gal + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * 1. Redistributions of source code must retain the above copyright notice, this + * list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED + * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE + * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR + * ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + * The views and conclusions contained in the software and documentation are those + * of the authors and should not be interpreted as representing official policies, + * either expressed or implied, of the Shogun Development Team. + */ + +#ifndef __CPU_INFO_H__ +#define __CPU_INFO_H__ + +#include + +SG_FORCED_INLINE static void CpuRelax() +{ +#ifdef _MSC_VER + _mm_pause(); +#else + asm("pause"); +#endif +} + +#endif /* __CPU_INFO_H__ */ From 5f3e59919b884490de8a003318a44f128bbec3aa Mon Sep 17 00:00:00 2001 From: Viktor Gal Date: Tue, 28 Feb 2017 15:29:54 +0800 Subject: [PATCH 24/24] Fix typo in Distance.cpp in omp initialisation this caused serious data race condition --- src/shogun/distance/Distance.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shogun/distance/Distance.cpp b/src/shogun/distance/Distance.cpp index 7284d412299..5b00c2107e3 100644 --- a/src/shogun/distance/Distance.cpp +++ b/src/shogun/distance/Distance.cpp @@ -282,7 +282,7 @@ SGMatrix CDistance::get_distance_matrix() #pragma omp parallel shared(num_threads, step) { #ifdef HAVE_OPENMP - #pragma opm single + #pragma omp single { num_threads=omp_get_num_threads(); step=total_num/num_threads;