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.

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

Check variadic pattern arguments at compile time #356

Merged
merged 10 commits into from
May 30, 2017
8 changes: 8 additions & 0 deletions dash/include/dash/Exception.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,14 @@
throw(excep_type(os.str())); \
} while(0)

#define DASH_ASSERT_ALWAYS(expr) do { \
if (!(expr)) { \
DASH_THROW(dash::exception::AssertionFailed, \
"Assertion failed: " \
<< " " << __FILE__ << ":" << __LINE__); \
}\
} while(0)

#if defined(DASH_ENABLE_ASSERTIONS)

#define DASH_ASSERT(expr) do { \
Expand Down
244 changes: 160 additions & 84 deletions dash/include/dash/pattern/internal/PatternArguments.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#ifndef DASH__INTERNAL__PATTERN_ARGUMENTS_H_
#define DASH__INTERNAL__PATTERN_ARGUMENTS_H_

#include <utility>

#include <dash/Types.h>
#include <dash/Team.h>
#include <dash/Dimensional.h>
Expand All @@ -25,7 +27,7 @@ template<
dim_t NumDimensions,
typename IndexType = dash::default_index_t>
class PatternArguments {
private:
public:
Copy link
Member

Choose a reason for hiding this comment

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

Why public?

/// Derive size type from given signed index / ptrdiff type
typedef typename std::make_unsigned<IndexType>::type
SizeType;
Expand All @@ -51,12 +53,6 @@ class PatternArguments {
ViewSpec_t _viewspec;
/// Team containing all units to which pattern elements are mapped
Team * _team = nullptr;
/// Number of distribution specifying arguments in varargs
int _argc_dist = 0;
/// Number of size/extent specifying arguments in varargs
int _argc_size = 0;
/// Number of team specifying arguments in varargs
int _argc_team = 0;

public:
/**
Expand All @@ -71,27 +67,14 @@ class PatternArguments {
*/
template<typename ... Args>
PatternArguments(Args && ... args) {
static_assert(
sizeof...(Args) >= NumDimensions,
"Invalid number of arguments for PatternArguments");
// Parse argument list:
check_recurse<0>(std::forward<Args>(args)...);
// Validate number of arguments after parsing:
if (_argc_size > 0 && _argc_size != NumDimensions) {
DASH_THROW(
dash::exception::InvalidArgument,
"Invalid number of size arguments for BlockPattern(...), " <<
"expected " << NumDimensions << ", got " << _argc_size);
}
if (_argc_dist > 0 && _argc_dist != NumDimensions) {
DASH_THROW(
dash::exception::InvalidArgument,
"Invalid number of dist arguments for BlockPattern(...), " <<
"expected " << NumDimensions << ", got " << _argc_dist);
}
check_tile_constraints();
parse<0, 0, 0, 0>(std::forward<Args>(args)...);
}


public:


bool is_tiled() const {
for (auto d = 0; d < NumDimensions; ++d) {
if (_distspec[d].is_tiled()) {
Expand Down Expand Up @@ -121,72 +104,163 @@ class PatternArguments {
}

private:
/// BlockPattern matching for extent value of type IndexType.
template<int count>
void check(SizeType extent) {
/*
* Match for up to \c NumDimensions extent value of type SizeType.
*
* \tparam ArgcSize The number of arguments describing the extents of the
* pattern parsed so far.
* Set to -1 if a \c SizeSpec is encountered.
* \tparam ArgcDist The number of arguments describing the distribution of
* the pattern parsed so far.
* Set to -1 if a \c DistributionSpec is encountered.
* \tparam ArgcTeam The number of arguments describing the team/unit
* distribution of the pattern parsed so far.
* Set to -1 if a \c TeamSpec is encountered.
*/
template<
int ArgcSize,
int ArgcDist,
int ArgcTeam,
int ArgcTeamSpec,
typename ... Args>
void parse(SizeType extent, Args && ... args) {
static_assert(ArgcSize >= 0, "Cannot mix size and SizeSpec definition"
"in variadic pattern constructor!");
DASH_LOG_TRACE("PatternArguments.check(extent)", extent);
_argc_size++;
_sizespec.resize(count, extent);
static_assert(ArgcSize < NumDimensions, "Number of size specifier exceeds"
"the number of dimensions in variadic pattern constructor!");
DASH_LOG_TRACE("PatternArguments.check(extent)", extent);
_sizespec.resize(ArgcSize, extent);
parse<ArgcSize+1, ArgcDist, ArgcTeam,
ArgcTeamSpec>(std::forward<Args>(args)...);
}

/*
* Match for one optional parameter specifying the size (extents).
*/
template<
int ArgcSize,
int ArgcDist,
int ArgcTeam,
int ArgcTeamSpec,
typename ... Args>
void parse(const SizeSpec_t & sizeSpec, Args && ... args) {
static_assert(ArgcSize == 0, "Cannot mix size and SizeSpec definition"
"in variadic pattern constructor!");
DASH_LOG_TRACE("PatternArguments.check(sizeSpec)");
_sizespec = sizeSpec;
parse<-1, ArgcDist, ArgcTeam,
ArgcTeamSpec>(std::forward<Args>(args)...);
}
/// BlockPattern matching for up to \c NumDimensions optional
/// parameters specifying the distribution pattern.
template<int count>
void check(const TeamSpec_t & teamSpec) {

/*
* Match for \c TeamSpec describing the distribution among the units
* in the team.
*/
template<
int ArgcSize,
int ArgcDist,
int ArgcTeam,
int ArgcTeamSpec,
typename ... Args>
void parse(const TeamSpec_t & teamSpec, Args && ... args) {
static_assert(ArgcTeamSpec == 0, "Cannot specify TeamSpec twice in "
"variadic pattern constructor!");
DASH_LOG_TRACE("PatternArguments.check(teamSpec)");
_argc_team++;
// TODO: there is no way to check whether this TeamSpec
// was created from the team provided in the variadic arguments.
_teamspec = teamSpec;
parse<ArgcSize, ArgcDist, ArgcTeam, 1>(std::forward<Args>(args)...);
}
/// BlockPattern matching for one optional parameter specifying the
/// team.
template<int count>
void check(dash::Team & team) {

/*
* Match for one optional parameter specifying the team.
*/
template<
int ArgcSize,
int ArgcDist,
int ArgcTeam,
int ArgcTeamSpec,
typename ... Args>
void parse(dash::Team & team, Args && ... args) {
static_assert(!(ArgcTeam > 0),
"Cannot specify Team twice in variadic "
"pattern constructor!");
DASH_LOG_TRACE("PatternArguments.check(team)");
if (_argc_team == 0) {
_team = &team;
_teamspec = TeamSpec_t(_distspec, team);
}
// assign team, TeamSpec will be created when parsing is finished
// and no TeamSpec has been found.
_team = &team;
parse<ArgcSize, ArgcDist, 1,
ArgcTeamSpec>(std::forward<Args>(args)...);
}
/// BlockPattern matching for one optional parameter specifying the
/// size (extents).
template<int count>
void check(const SizeSpec_t & sizeSpec) {
DASH_LOG_TRACE("PatternArguments.check(sizeSpec)");
_argc_size += NumDimensions;
_sizespec = sizeSpec;
}
/// BlockPattern matching for one optional parameter specifying the
/// distribution.
template<int count>
void check(const DistributionSpec_t & ds) {

/*
* Match for one optional parameter specifying the distribution.
*/
template<
int ArgcSize,
int ArgcDist,
int ArgcTeam,
int ArgcTeamSpec,
typename ... Args>
void parse(const DistributionSpec_t & ds, Args && ... args) {
static_assert(ArgcDist == 0, "Cannot mix DistributionSpec and inidividual"
"distributions in variadic pattern constructor!");
DASH_LOG_TRACE("PatternArguments.check(distSpec)");
_argc_dist += NumDimensions;
_distspec = ds;
_distspec = ds;
parse<ArgcSize, -1, ArgcTeam,
ArgcTeam>(std::forward<Args>(args)...);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the use of ArgcTeam instead of ArgcTeamSpec intended here?

}
/// BlockPattern matching for up to NumDimensions optional parameters
/// specifying the distribution.
template<int count>
void check(const Distribution & ds) {

/*
* Match for up to \c NumDimensions optional parameters
* specifying the distribution.
*/
template<
int ArgcSize,
int ArgcDist,
int ArgcTeam,
int ArgcTeamSpec,
typename ... Args>
void parse(const Distribution & ds, Args && ... args) {
static_assert(!(ArgcDist < 0), "Cannot mix DistributionSpec and "
"inidividual distributions in variadic pattern constructor!");
static_assert(ArgcDist < NumDimensions, "Number of distribution specifier"
" exceeds the number of dimensions in variadic pattern constructor!");
DASH_LOG_TRACE("PatternArguments.check(dist)");
_argc_dist++;
dim_t dim = count - NumDimensions;
_distspec[dim] = ds;
}
/// Isolates first argument and calls the appropriate check() function
/// on each argument via recursion on the argument list.
template<int count, typename T, typename ... Args>
void check_recurse(T && t, Args && ... args) {
DASH_LOG_TRACE("PatternArguments.check(args) ",
"count", count,
"argc", sizeof...(Args));
check<count>(std::forward<T>(t));
if (sizeof...(Args) > 0) {
check_recurse<count + 1>(std::forward<Args>(args)...);
}
_distspec[ArgcDist] = ds;
parse<ArgcSize, ArgcDist+1, ArgcTeam,
ArgcTeam>(std::forward<Args>(args)...);
Copy link
Contributor

Choose a reason for hiding this comment

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

Here too.

}
/// Terminator function for recursive argument parsing
template<int count>
void check_recurse() {

/**
* Stop recursion when all arguments are processed.
*/
template<
int ArgcSize,
int ArgcDist,
int ArgcTeam,
int ArgcTeamSpec>
void parse() {
static_assert(!(ArgcDist > 0 && ArgcDist != NumDimensions),
"Incomplete distribution specification in "
"variadic pattern constructor!");

static_assert(!(ArgcSize > 0 && ArgcSize != NumDimensions),
"Incomplete size specification in "
"variadic pattern constructor!");

/*
* TODO: we have no way to statically check whether a TeamSpec was created
* using the team specified by the user.
*/
if (ArgcTeam && !ArgcTeamSpec) {
_teamspec = TeamSpec_t(_distspec, *_team);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is possible to return without _team set, how about:

if(!ArgcTeam) {
  _team = &dash::Team::All()
}
if(!ArgcTeamSpec) {
  _teamspec = TeamSpec_t(_distspec, *_team);
}

I'm not sure about the use cases of this constructor,
so perhaps it is also fine to return without a team.

Copy link
Member Author

Choose a reason for hiding this comment

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

The dash::Team::All is used by default if no team has been set, see team(). The assumption here is that it only makes sense to overwrite the default TeamSpec if a team was set. However, I realized that I am not sure about the relation between DistSpec and TeamSpec and whether we have to create the latter if the former was explicitely given bu the user. Is it even valid to specify both at the same time? @fuchsto can you shed some light?


check_tile_constraints();
}

/// Check pattern constraints for tile
void check_tile_constraints() const {
bool has_tile = false;
Expand All @@ -197,20 +271,22 @@ class PatternArguments {
if (_distspec.dim(i).type != _distspec.dim(i+1).type)
invalid = true;
}
if (has_tile && invalid) {
DASH_THROW(dash::exception::InvalidArgument,
"Pattern arguments invalid: Mixed distribution types");
}
if (has_tile) {
if (invalid) {
DASH_THROW(dash::exception::InvalidArgument,
"Pattern arguments invalid: Mixed distribution types");
}

for (auto i = 0; i < NumDimensions; i++) {
assert(
DASH_ASSERT_ALWAYS(
_sizespec.extent(i) % (_distspec.dim(i).blocksz)
== 0);
}
}
}
};


} // namespace internal
} // namespace dash

Expand Down
19 changes: 19 additions & 0 deletions dash/test/pattern/BlockPatternTest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ TEST_F(BlockPatternTest, SimpleConstructor)
EXPECT_EQ(dash::Team::All().size(), pat_default.num_units());
EXPECT_EQ(size, pat_default.capacity());

dash::SizeSpec<3> sspec(
extent_x, extent_y, extent_z);
dash::DistributionSpec<3> ds_blocked_z(
dash::NONE, dash::NONE, dash::BLOCKED);
dash::BlockPattern<3, dash::COL_MAJOR> pat_ds(
Expand All @@ -44,6 +46,23 @@ TEST_F(BlockPatternTest, SimpleConstructor)
EXPECT_EQ(ds_blocked_z, pat_ds_t.distspec());
EXPECT_EQ(size, pat_ds_t.capacity());
EXPECT_EQ(dash::Team::All().size(), pat_ds_t.num_units());

// As above but mix size and distribution specification.
// Has to start with a size specifier.
dash::BlockPattern<3> pat_mixed(
extent_x, dash::NONE, extent_y, dash::NONE, extent_z,
dash::BLOCKED,
dash::Team::All());
EXPECT_EQ(ds_blocked_z, pat_ds_t.distspec());
EXPECT_EQ(size, pat_ds_t.capacity());
EXPECT_EQ(dash::Team::All().size(), pat_ds_t.num_units());

dash::BlockPattern<3> pat_spec(
sspec, ds_blocked_z,
dash::Team::All());
EXPECT_EQ(ds_blocked_z, pat_spec.distspec());
EXPECT_EQ(size, pat_spec.capacity());
EXPECT_EQ(dash::Team::All().size(), pat_spec.num_units());
}

TEST_F(BlockPatternTest, EqualityComparison)
Expand Down