Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Vectorized RNG #4437

Closed
wants to merge 7 commits into from
Closed

Vectorized RNG #4437

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
424e61e
Vectorized RNG in SGMatrix
saatvikshah Dec 15, 2018
0c59ece
SGVector Vectorized RNG
saatvikshah Dec 19, 2018
8c7fe7f
Apply Formatting changes
saatvikshah Dec 19, 2018
e0bd50e
Merge remote-tracking branch 'upstream/develop' into feature/vectoriz…
saatvikshah Jan 5, 2019
62c88cd
Addressed review comments
saatvikshah Jan 5, 2019
823e70d
Adding benchmark
saatvikshah Jan 11, 2019
6a97846
sg_rand usage for SGVector
saatvikshah Jan 12, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/shogun/base/SGObject.h
View file
Open in desktop
Expand Up @@ -13,7 +13,6 @@

#include <shogun/base/AnyParameter.h>
#include <shogun/base/Version.h>
#include <shogun/base/base_types.h>
#include <shogun/base/some.h>
#include <shogun/base/unique.h>
#include <shogun/io/SGIO.h>
Expand All @@ -25,6 +24,7 @@
#include <shogun/lib/exception/ShogunException.h>
#include <shogun/lib/parameter_observers/ObservedValue.h>
#include <shogun/lib/tag.h>
#include <shogun/lib/sg_type_traits.h>

#include <utility>
#include <vector>
Expand Down
40 changes: 0 additions & 40 deletions src/shogun/base/base_types.h
View file
Open in desktop

This file was deleted.

11 changes: 6 additions & 5 deletions src/shogun/lib/SGMatrix.cpp
View file
Open in desktop
Expand Up @@ -7,16 +7,16 @@
* Koen van de Sande, Roman Votyakov
*/

#include <shogun/lib/config.h>
#include <shogun/io/SGIO.h>
#include <algorithm>
#include <limits>
#include <shogun/io/File.h>
#include <shogun/io/SGIO.h>
#include <shogun/lib/SGMatrix.h>
#include <shogun/lib/SGVector.h>
#include <shogun/mathematics/eigen3.h>
#include <shogun/lib/config.h>
#include <shogun/mathematics/Math.h>
#include <shogun/mathematics/eigen3.h>
#include <shogun/mathematics/lapack.h>
#include <limits>
#include <algorithm>

namespace shogun
{
Expand Down Expand Up @@ -1193,6 +1193,7 @@ SGVector<T> SGMatrix<T>::get_diagonal_vector() const
return diag;
}

// Explicit Specialization of Templates
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unneeded - will revert

template class SGMatrix<bool>;
template class SGMatrix<char>;
template class SGMatrix<int8_t>;
Expand Down
62 changes: 58 additions & 4 deletions src/shogun/lib/SGMatrix.h
View file
Open in desktop
Expand Up @@ -11,13 +11,16 @@

#include <shogun/base/macros.h>
#include <shogun/io/SGIO.h>
#include <shogun/lib/config.h>
#include <shogun/lib/SGReferencedData.h>
#include <shogun/lib/common.h>
#include <shogun/lib/config.h>
#include <shogun/mathematics/Random.h>
#include <shogun/util/iterators.h>
#include <shogun/lib/SGReferencedData.h>

#include <memory>
#include "sg_type_traits.h"
#include <atomic>
#include <memory>
#include <algorithm>

namespace Eigen
{
Expand Down Expand Up @@ -306,9 +309,60 @@ template<class T> class SGMatrix : public SGReferencedData
/** Set matrix to a constant */
void set_const(T const_elem);

/** fill matrix with zeros */
/** Fill matrix with zeros */
void zero();

/**
iglesias marked this conversation as resolved.
Show resolved Hide resolved
* Fill matrix with uniformly distributed randoms
* Float Types: [0, 1)
* Int Types: std::numeric_limit<T>::(min->max)
**/
template <typename U>
using enable_simd_float = std::enable_if_t<sg_is_same_v<U, float64_t>>;
template <typename U = T>
auto random() -> enable_simd_float<U>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

separating declaration and definition would need explicit specialization of each template method signature - which would end up being really long. Since the methods are relatively short it seemed ok to keep both in the header file.

The only disadvantage is that an additional include of Math.h has to be added here

Copy link
Member

@gf712 gf712 Dec 17, 2018 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So is this the same as writing:

template <typename T>
typename std::enable_if_t<is_any<T, float64_t>::value, void>>
random()
{
...
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep - Actually 3 versions are possible which mean the same:

  1. https://coliru.stacked-crooked.com/a/bc08035924d3582e
  2. https://coliru.stacked-crooked.com/a/7cc61a4adcdd5ded
  3. https://coliru.stacked-crooked.com/a/45dba741ddf2d4a0

Mine is a slightly more readable version of (2) :D

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK! It is more readable indeed! Why do you need the template <typename U = T> though? Doesn't template <typename T> work?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SFINAE only works on templated functions. A method inside a template class doesnt count. The function itself has to also be templated. As mentioned in CPPReference This rule applies during overload resolution of function templates

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw I wrote up a short blog post on this topic - more as a point of reference but here it is in case anyone is interested :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah true! Thanks, I'll have a look!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah cool!

{
auto r = std::make_unique<CRandom>();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the problem with having CRandom created here is that you wont ever be able to set seed :(
there's a way to do this, check the usage of the global variable sg_rand in the codebase... but i rather would start reviving the feature/random-refactor branch and have then this fixed....

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the rest of the patch has the same pattern and it'll be a problem as well there...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ive addressed it for SGMatrix
SGVector/CRandom are being included in one another, which is causing a problem when using sg_rand in the header file directly. I'll have a go at it tomorrow.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice!

r->fill_array_co(matrix, num_rows * num_cols);
}

template <typename U>
using enable_simd_int =
std::enable_if_t<sg_is_any_of_v<U, uint32_t, uint64_t>>;
template <typename U = T>
auto random() -> enable_simd_int<U>
{
auto r = std::make_unique<CRandom>();
r->fill_array(matrix, num_rows * num_cols);
}

template <typename U>
using enable_nonsimd_float =
std::enable_if_t<sg_is_any_of_v<U, float32_t, floatmax_t>>;
template <typename U = T>
auto random() -> enable_nonsimd_float<U>
{
auto r = std::make_unique<CRandom>();
// Casting floats upwards or downwards is safe
// Ref: https://stackoverflow.com/a/36840390/3656081
std::transform(matrix, matrix + (num_rows*num_cols), matrix,
[&r](auto){ return r->random_half_open(); });
}

template <typename U>
using enable_nonsimd_int =
std::enable_if_t<sg_is_any_of_v<U, int8_t, uint8_t, int16_t, uint16_t,
int32_t, int64_t>>;
template <typename U = T>
auto random() -> enable_nonsimd_int<U>
{
auto r = std::make_unique<CRandom>();
std::transform(matrix, matrix + (num_rows*num_cols), matrix,
[&r](auto){ return r->random(
std::numeric_limits<T>::min(),
std::numeric_limits<T>::max()); });
}

/**
* Checks whether the matrix is symmetric or not. The equality check
* is performed using '==' operators for discrete types (int, char,
Expand Down
59 changes: 56 additions & 3 deletions src/shogun/lib/SGVector.h
View file
Open in desktop
Expand Up @@ -13,14 +13,16 @@

#include <shogun/base/macros.h>
#include <shogun/io/SGIO.h>
#include <shogun/lib/common.h>
#include <shogun/lib/SGReferencedData.h>
#include <shogun/util/iterators.h>
#include <shogun/lib/common.h>
#include <shogun/mathematics/Random.h>
#include <shogun/mathematics/linalg/GPUMemoryBase.h>
#include <shogun/util/iterators.h>

#include <memory>
#include "sg_type_traits.h"
#include <atomic>
#include <initializer_list>
#include <memory>

namespace Eigen
{
Expand Down Expand Up @@ -204,6 +206,57 @@ template<class T> class SGVector : public SGReferencedData
/** Fill vector with zeros */
void zero();

/**
* Fill matrix with uniformly distributed randoms
* Float Types: [0, 1)
* Int Types: std::numeric_limit<T>::(min->max)
**/
template <typename U>
using enable_simd_float = std::enable_if_t<sg_is_same_v<U, float64_t>>;
template <typename U = T>
auto random() -> enable_simd_float<U>
{
auto r = std::make_unique<CRandom>();
r->fill_array_co(vector, vlen);
}

template <typename U>
using enable_simd_int =
std::enable_if_t<sg_is_any_of_v<U, uint32_t, uint64_t>>;
template <typename U = T>
auto random() -> enable_simd_int<U>
{
auto r = std::make_unique<CRandom>();
r->fill_array(vector, vlen);
}

template <typename U>
using enable_nonsimd_float =
std::enable_if_t<sg_is_any_of_v<U, float32_t, floatmax_t>>;
template <typename U = T>
auto random() -> enable_nonsimd_float<U>
{
auto r = std::make_unique<CRandom>();
// Casting floats upwards or downwards is safe
// Ref: https://stackoverflow.com/a/36840390/3656081
std::transform(vector, vector + vlen, vector,
[&r](auto){ return r->random_half_open(); });
}

template <typename U>
using enable_nonsimd_int =
std::enable_if_t<sg_is_any_of_v<U, int8_t, uint8_t, int16_t, uint16_t,
int32_t, int64_t>>;
template <typename U = T>
auto random() -> enable_nonsimd_int<U>
{
auto r = std::make_unique<CRandom>();
std::transform(vector, vector + vlen, vector,
[&r](auto){ return r->random(
std::numeric_limits<T>::min(),
std::numeric_limits<T>::max()); });
}

/** Range fill a vector with start...start+len-1
*
* @param start - value to be assigned to first element of vector
Expand Down
58 changes: 58 additions & 0 deletions src/shogun/lib/sg_type_traits.h
View file
Open in desktop
@@ -0,0 +1,58 @@
/*
* This software is distributed under BSD 3-clause license (see LICENSE file).
*
* Authors: Saatvik Shah
*/

#include <type_traits>

#ifndef SHOGUN_SG_TYPE_TRAITS_H
#define SHOGUN_SG_TYPE_TRAITS_H
/**
* A collection of useful type traits
*/

namespace shogun {
template <typename T1, typename T2>
constexpr bool sg_is_same_v = std::is_same<T1, T2>::value;
// Checks if any one of the types matches
// Ref: https://stackoverflow.com/a/17032517/3656081
template<typename T, typename... Rest>
struct sg_is_any_of : std::false_type {
};

template<typename T, typename First>
struct sg_is_any_of<T, First> : std::is_same<T, First> {
};

template<typename T, typename First, typename... Rest>
struct sg_is_any_of<T, First, Rest...>
: std::integral_constant<bool, std::is_same<T, First>::value ||
sg_is_any_of<T, Rest...>::value> {
};

template<typename T, typename First, typename... Rest>
constexpr bool sg_is_any_of_v = sg_is_any_of<T, First, Rest...>::value;

// all shogun base classes for put/add templates
class CMachine;
class CKernel;
class CDistance;
class CFeatures;
class CLabels;
class CECOCEncoder;
class CECOCDecoder;
class CMulticlassStrategy;
class CNeuralLayer;
// type trait to enable certain methods only for shogun base types
template<class T>
struct is_sg_base
: std::integral_constant<bool,
sg_is_any_of_v<T, CMachine, CKernel,
CDistance, CFeatures,
CLabels, CECOCEncoder,
CECOCDecoder, CMulticlassStrategy,
CNeuralLayer>> {
};
}
#endif // SHOGUN_SG_TYPE_TRAITS_H
16 changes: 16 additions & 0 deletions tests/unit/lib/SGMatrix_unittest.cc
View file
Open in desktop
Expand Up @@ -13,6 +13,22 @@

using namespace shogun;

TEST(SGMatrixTest, vectorized_rng)
{
auto rng_check_float = [](auto elem) {
SGMatrix<decltype(elem)> sg_mat(10, 5);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would there be a away to check it against the individual calling of random with some seed fixing magic?
If that's a pain, dont bother, this test is more to check that no memory errors happen I guess

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might be a bit of a chore to implement - especially because I'd have to based on the type use the random_half_open() vs. fill_array_oc version.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah don't worry then

sg_mat.random();
for (int i = 0; i < sg_mat.size(); i++)
{
EXPECT_LE(0.0, sg_mat[i]);
EXPECT_GT(1.0, sg_mat[i]);
}
};
rng_check_float(float32_t{0.0});
rng_check_float(float64_t{0.0});
rng_check_float(floatmax_t{0.0});
}

TEST(SGMatrixTest,ctor_zero_const)
{
SGMatrix<float64_t> a(10, 5);
Expand Down
16 changes: 16 additions & 0 deletions tests/unit/lib/SGVector_unittest.cc
View file
Open in desktop
Expand Up @@ -9,6 +9,22 @@

using namespace shogun;

TEST(SGVectorTest, vectorized_rng)
{
auto rng_check_float = [](auto elem) {
SGVector<decltype(elem)> sg_vec(50);
sg_vec.random();
for (int i = 0; i < sg_vec.size(); i++)
{
EXPECT_LE(0.0, sg_vec[i]);
EXPECT_GT(1.0, sg_vec[i]);
}
};
rng_check_float(float32_t{0.0});
rng_check_float(float64_t{0.0});
rng_check_float(floatmax_t{0.0});
}

TEST(SGVectorTest,ctor)
{
SGVector<float64_t> a(10);
Expand Down