Skip to content

Commit

Permalink
review updates
Browse files Browse the repository at this point in the history 
Co-authored-by: Thomas Grützmacher <thomas.gruetzmacher@kit.edu>
Co-authored-by: Terry Cojean <terry.cojean@kit.edu>
Co-authored-by: Yuhsiang M. Tsai <yhmtsai@gmail.com>
  • Loading branch information
@upsj @tcojean @yhmtsai
4 people committed Aug 13, 2020
1 parent 4c4e44e commit 3ade128
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 39 deletions.
34 changes: 17 additions & 17 deletions cuda/test/base/lin_op.cu
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,27 +44,27 @@ protected:
FactoryParameter() {}

public:
std::vector<int> GKO_FACTORY_PARAMETER_VECTOR(parameter, 10, 11);
int GKO_FACTORY_PARAMETER_SCALAR(parameter2, -4);
std::vector<int> GKO_FACTORY_PARAMETER_VECTOR(vector_parameter, 10, 11);
int GKO_FACTORY_PARAMETER_SCALAR(scalar_parameter, -4);
};


TEST_F(FactoryParameter, WorksOnCudaDefault)
{
std::vector<int> expected{10, 11};

ASSERT_EQ(parameter, expected);
ASSERT_EQ(parameter2, -4);
ASSERT_EQ(vector_parameter, expected);
ASSERT_EQ(scalar_parameter, -4);
}


TEST_F(FactoryParameter, WorksOnCuda0)
{
std::vector<int> expected{};

auto result = &this->with_parameter();
auto result = &this->with_vector_parameter();

ASSERT_EQ(parameter, expected);
ASSERT_EQ(vector_parameter, expected);
ASSERT_EQ(result, this);
}

Expand All @@ -73,50 +73,50 @@ TEST_F(FactoryParameter, WorksOnCuda1)
{
std::vector<int> expected{2};

this->with_parameter(2).with_parameter2(3);
this->with_vector_parameter(2).with_scalar_parameter(3);

ASSERT_EQ(parameter, expected);
ASSERT_EQ(parameter2, 3);
ASSERT_EQ(vector_parameter, expected);
ASSERT_EQ(scalar_parameter, 3);
}


TEST_F(FactoryParameter, WorksOnCuda2)
{
std::vector<int> expected{8, 3};

this->with_parameter(8, 3);
this->with_vector_parameter(8, 3);

ASSERT_EQ(parameter, expected);
ASSERT_EQ(vector_parameter, expected);
}


TEST_F(FactoryParameter, WorksOnCuda3)
{
std::vector<int> expected{1, 7, 2};

this->with_parameter(1, 7, 2);
this->with_vector_parameter(1, 7, 2);

ASSERT_EQ(parameter, expected);
ASSERT_EQ(vector_parameter, expected);
}


TEST_F(FactoryParameter, WorksOnCuda4)
{
std::vector<int> expected{4, 5, 4, 2};

this->with_parameter(4, 5, 4, 2);
this->with_vector_parameter(4, 5, 4, 2);

ASSERT_EQ(parameter, expected);
ASSERT_EQ(vector_parameter, expected);
}


TEST_F(FactoryParameter, WorksOnCuda5)
{
std::vector<int> expected{9, 3, 4, 2, 7};

this->with_parameter(9, 3, 4, 2, 7);
this->with_vector_parameter(9, 3, 4, 2, 7);

ASSERT_EQ(parameter, expected);
ASSERT_EQ(vector_parameter, expected);
}


Expand Down
34 changes: 17 additions & 17 deletions hip/test/base/lin_op.hip.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,27 +44,27 @@ class FactoryParameter : public ::testing::Test {
FactoryParameter() {}

public:
std::vector<int> GKO_FACTORY_PARAMETER_VECTOR(parameter, 10, 11);
int GKO_FACTORY_PARAMETER_SCALAR(parameter2, -4);
std::vector<int> GKO_FACTORY_PARAMETER_VECTOR(vector_parameter, 10, 11);
int GKO_FACTORY_PARAMETER_SCALAR(scalar_parameter, -4);
};


TEST_F(FactoryParameter, WorksOnHipDefault)
{
std::vector<int> expected{10, 11};

ASSERT_EQ(parameter, expected);
ASSERT_EQ(parameter2, -4);
ASSERT_EQ(vector_parameter, expected);
ASSERT_EQ(scalar_parameter, -4);
}


TEST_F(FactoryParameter, WorksOnHip0)
{
std::vector<int> expected{};

auto result = &this->with_parameter();
auto result = &this->with_vector_parameter();

ASSERT_EQ(parameter, expected);
ASSERT_EQ(vector_parameter, expected);
ASSERT_EQ(result, this);
}

Expand All @@ -73,50 +73,50 @@ TEST_F(FactoryParameter, WorksOnHip1)
{
std::vector<int> expected{2};

this->with_parameter(2).with_parameter2(3);
this->with_vector_parameter(2).with_scalar_parameter(3);

ASSERT_EQ(parameter, expected);
ASSERT_EQ(parameter2, 3);
ASSERT_EQ(vector_parameter, expected);
ASSERT_EQ(scalar_parameter, 3);
}


TEST_F(FactoryParameter, WorksOnHip2)
{
std::vector<int> expected{8, 3};

this->with_parameter(8, 3);
this->with_vector_parameter(8, 3);

ASSERT_EQ(parameter, expected);
ASSERT_EQ(vector_parameter, expected);
}


TEST_F(FactoryParameter, WorksOnHip3)
{
std::vector<int> expected{1, 7, 2};

this->with_parameter(1, 7, 2);
this->with_vector_parameter(1, 7, 2);

ASSERT_EQ(parameter, expected);
ASSERT_EQ(vector_parameter, expected);
}


TEST_F(FactoryParameter, WorksOnHip4)
{
std::vector<int> expected{4, 5, 4, 2};

this->with_parameter(4, 5, 4, 2);
this->with_vector_parameter(4, 5, 4, 2);

ASSERT_EQ(parameter, expected);
ASSERT_EQ(vector_parameter, expected);
}


TEST_F(FactoryParameter, WorksOnHip5)
{
std::vector<int> expected{9, 3, 4, 2, 7};

this->with_parameter(9, 3, 4, 2, 7);
this->with_vector_parameter(9, 3, 4, 2, 7);

ASSERT_EQ(parameter, expected);
ASSERT_EQ(vector_parameter, expected);
}


Expand Down
14 changes: 11 additions & 3 deletions include/ginkgo/core/base/lin_op.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -754,6 +754,9 @@ public: \
* // a factory parameter named "my_value", of type int and default
* // value of 5
* int GKO_FACTORY_PARAMETER_SCALAR(my_value, 5);
* // a factory parameter named `my_pair` of type `std::pair<int,int>`
* // and default value {5, 5}
* std::pair<int, int> GKO_FACTORY_PARAMETER_VECTOR(my_pair, 5, 5);
* };
* // constructor needed by EnableLinOp
* explicit MyLinOp(std::shared_ptr<const Executor> exec) {
Expand Down Expand Up @@ -888,6 +891,7 @@ public: \
static_assert(true, \
"This assert is used to counter the false positive extra " \
"semi-colon warnings")

/**
* Creates a scalar factory parameter in the factory parameters structure.
*
Expand All @@ -903,6 +907,7 @@ public: \
*/
#define GKO_FACTORY_PARAMETER_SCALAR(_name, _default) \
GKO_FACTORY_PARAMETER(_name, _default)

/**
* Creates a vector factory parameter in the factory parameters structure.
*
Expand All @@ -921,7 +926,8 @@ public: \
#else // defined(__CUDACC__) || defined(__HIPCC__)
// A workaround for the NVCC compiler - parameter pack expansion does not work
// properly, because while the assignment to a scalar value is translated by
// cudafe without parameter pack expansion, the corresponding parameter is not.
// cudafe into a C-style cast, the parameter pack expansion is not removed and
// `Args&&... args` is still kept as a parameter pack.
#define GKO_FACTORY_PARAMETER(_name, ...) \
mutable _name{__VA_ARGS__}; \
\
Expand All @@ -935,12 +941,13 @@ public: \
static_assert(true, \
"This assert is used to counter the false positive extra " \
"semi-colon warnings")

#define GKO_FACTORY_PARAMETER_SCALAR(_name, _default) \
mutable _name{_default}; \
\
template <typename Arg> \
auto with_##_name(Arg &&_value) \
const->const ::gko::xstd::decay_t<decltype(*this)> & \
const->const std::decay_t<decltype(*this)> & \
{ \
using type = decltype(this->_name); \
this->_name = type{std::forward<Arg>(_value)}; \
Expand All @@ -949,12 +956,13 @@ public: \
static_assert(true, \
"This assert is used to counter the false positive extra " \
"semi-colon warnings")

#define GKO_FACTORY_PARAMETER_VECTOR(_name, ...) \
mutable _name{__VA_ARGS__}; \
\
template <typename... Args> \
auto with_##_name(Args &&... _value) \
const->const ::gko::xstd::decay_t<decltype(*this)> & \
const->const std::decay_t<decltype(*this)> & \
{ \
using type = decltype(this->_name); \
this->_name = type{std::forward<Args>(_value)...}; \
Expand Down
4 changes: 2 additions & 2 deletions include/ginkgo/core/preconditioner/jacobi.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ class Jacobi : public EnableLinOp<Jacobi<ValueType, IndexType>>,
* has to be respected when setting this parameter. Failure to do
* so will lead to undefined behavior.
*/
gko::Array<index_type> GKO_FACTORY_PARAMETER_SCALAR(block_pointers,
gko::Array<index_type> GKO_FACTORY_PARAMETER_VECTOR(block_pointers,
nullptr);

private:
Expand Down Expand Up @@ -436,7 +436,7 @@ class Jacobi : public EnableLinOp<Jacobi<ValueType, IndexType>>,
* If the non-adaptive version of Jacobi is used, the
* `storage_optimization.block_wise` Array will be empty.
*/
storage_optimization_type GKO_FACTORY_PARAMETER_SCALAR(
storage_optimization_type GKO_FACTORY_PARAMETER_VECTOR(
storage_optimization, precision_reduction(0, 0));

/**
Expand Down

0 comments on commit 3ade128

Please sign in to comment.