arcosuc3m/grppi

Generic Reusable Parallel Patterns Interface

grppi's Issues

File name conventions

We will follow file name conventions:

• Headers whatever.h
• Implementation whatever.cpp

Please see http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#S-source

Consistency in include guards

According to CppCoreGuidelines SF.8 all include guards should have the form

GRPPI_FILENAME_H

if the file was named filename.h

Polymorphic map - Support for several input arguments

The polymorphic map does not support at the moment the use of the map patter with two or more input arguments.

For example, following code cannot be compiled:
grppi::map(e, begin(in), end(in), begin(out),
[&](int i, int j) { return i*constant + j; }, begin(in2) );

Being in, out and in2 vectors of integers with the same length.

Change TaskFunc name

Template argument TaskFunc should be changes to better align with STL

• Operation
• UnaryOperation
• BinaryOperation

Problems compiling with GCC 6.1

/home/fjblas/grppi/tests/pipeline+farm.cpp:100:5: required from here
/home/software/gcc-6.1/include/c++/6.1.0/bits/stl_pair.h:224:17: internal compiler error: in cp_genericize, at cp/cp-gimplify.c:1567
constexpr pair(const pair&) = default;
^~~~
0x6e8e9d cp_genericize(tree_node*)
../.././gcc/cp/cp-gimplify.c:1567
0x5f26f9 finish_function(int)
../.././gcc/cp/decl.c:14820
0x69d77e synthesize_method(tree_node*)
../.././gcc/cp/method.c:817
0x63d0c0 mark_used(tree_node*, int)
../.././gcc/cp/decl2.c:5251
0x5d3d3a build_over_call
../.././gcc/cp/call.c:7608
0x5d255f build_new_method_call_1
../.././gcc/cp/call.c:8419
0x5d255f build_new_method_call(tree_node*, tree_node*, vec<tree_node*, va_gc, vl_embed>, tree_node*, int, tree_node, int)
../.././gcc/cp/call.c:8489
0x69e34e locate_fn_flags
../.././gcc/cp/method.c:900
0x69e516 get_copy_ctor(tree_node*, int)
../.././gcc/cp/method.c:958
0x6e578b omp_cxx_notice_variable
../.././gcc/cp/cp-gimplify.c:908
0x6e7034 cp_genericize_r
../.././gcc/cp/cp-gimplify.c:1018

Add CMake support for documentation generation

We should support doxygen generation as a target for CMake

Change naming convention

We need to change the naming of the library interface to avoid CamelCase.

See: http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rl-camel

Functions should have names like:

• pipeline()
• divide_and_conquer()
• ...

Rename classes in patterns.h

PipelineObj -> pipeline_info
ReduceObj -> reduction_info
FarmObj -> farm_info
FilterObj -> filter_info

Back-end directories and file names in include

Rename directories in include

• ppi_seq -> seq
• ppi_omp -> omp
• ppi_thr -> native
• ppi_tbb -> tbb
• ppi_thrust -> thrust

In every directoy file names should not contain any backend-suffix.

Examples:

• ppi_seq/map_seq.h -> seq/map.h
• ppi_seq/reduce_seq.h -> seq/reduce.h
• ppi_omp/farm_omp.h -> omp/farm.h
• ...

Enforce maximum warning levels from compiler

We should have things like -Wall -Werror, ...

Revise the design of execution model

Is it really needed to have a class hierarchy?

Unit tests for static versions of map algorithm

Add a unit test file for map algorithm with all policies.

Revise documentation for map pattern

The documentation of map pattern needs to be revised to ensure sync with code.

Add basic numa infrastructure and improve thread policy

Create the infrastructure needed for adding NUMA support

Remove Thrust backend

Thrust back-end is outdated and needs a re-design.

For version 0.2 the decision is to remove.

Doxygen configuration

Doxygen configuration directory should named "doxy".

Generated files should be sent to directory "doc".

A README.md in the "doxy" directory should describe the contents of the directory.

What is the difference between "Doxyfile" and "Doxyfile.out". Should "Doxyfile.out" be under git?

Remove is_iterator.h

Move the code to map or remove it.

Add clang-tidy to unit-tests

Define an appropriate configuration file for clang-tidy and integrate clan-tidy checks in unit-tests

Rename pattern objects

PipelineObj -> pipeline_info
ReduceObj -> reduction_info
FarmObj -> farm_info
FilterObjt -> filter_info

Remove inline from function templates

Remove inline from all template functions

Move old gtests to legacy_utest

We will move all old unit_tests to legayc_utest directory.

This work is part of a deep revision of testing strategy.

Add support for GoogleTest under clang++

Currently GTest does not work on clang due to linking problems with coverage toolset.

Proposal: Remove coverage test of unit testing when using clang.

Redesigning map variadic interface

Currently the variadic interface for map is

map(exec, first1, last1, out, functor, first2, first3, ..., firstN)

This order is a bit chaotic

Alterante solution:

map(exec, make_tuple(first1, first2, ..., firstN), last1, out, functor);

Update all mentions in code of version

All source code an documentation should mention v0.2

ifdef for optional parts

Preprocessor ifdefs are now in every inclussion of optional components

For example:

#ifdef GRPPI_OMP
	#include "ppi_omp/map_omp.h"
#endif

This is error prone. Instead we will move those ifdefs to the included file.

For example:

//File: map_omp.h
#ifdef GRPPI_MAP_OMP_H
#define GRPPI_MAP_OMP_H

#ifdef GRPPI_OMP

namespace grppi {

// contents

} // end namespace grppi

#endif

#endif

No using namespace in headers

Remove all using namespace std in header files at global scope.

This is against CppCoreGuideline SF.7 (http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#sf7-dont-write-using-namespace-in-a-header-file)

SF.7: Don’t write using namespace in a header file
Reason Doing so takes away an #includer’s ability to effectively disambiguate and to use alternatives.

Function arguments by forwarding reference

Revise all patterns to take functors as forwarding references.

As an example:

template <typename InputIt, typename OutputIt, typename TaskFunc>
void map(parallel_execution_omp p, InputIt first, InputIt last, OutputIt firstOut, TaskFunc const &taskf)

should be transformed into

template <typename InputIt, typename OutputIt, typename TaskFunc>
void map(parallel_execution_omp p, InputIt first, InputIt last, OutputIt firstOut, TaskFunc && taskf)

Move cmake testing files to tests directory

There are CMake files in the root directory that are only used by the tests directory.

Move those files there.

Change unit tests guards

The unitary tests used the removed file "enable_flags" to enable the different policies tests.

The unitary tests files shall be modified to change the current guards TBB_ENABLE by GRPPI_TBB and OMP_ENABLE by GRPPI_OMP.

Revise config header

Revise the config header file and make sure if something is really needed

Rename include directory

Main include directory should be named "include"

Refactor thread_pool

1. Move source code to ppi_thr directory
2. Make all data members private and add needed member functions.

Add hwloc as externalproject_add

We need to depend on hwloc library through cmake's ExternalProject_Add().

This will allow depending on an specific version even if hwloc is not installed in the machine.

I propose depending on "hwloc-1.11.7".

Remove common/operator.h

It seems it is no longer needed

Algorithms and arity of functors

Algorithms should check the arity of the operation functor.

A simple compile error should be given, using a constraint (requires clause) or making use of a static_assert if the first option is problematic.

Add a policy for dynamic execution

Add a new policy that can hold any other policy

Remove map version with generators

The version of map that takes a generation function and a task function seems to be not necessary.

That seems to be a farm pattern.

I suggest to remove it from the map pattern interface.

Create samples for map pattern

Create a samples directory.

It shall have a directory per pattern. Add a directory for map pattern.

In "samples/map" add a directory per sample application.

Add sized interface for data parallel algorithms

Add an additional interfaces to data-parallel algorithms taking as an argument an integer size.

The following call should be valid

map(e, begin(v), n, begin(w), func);

There are two options for this:

• Name the function with a different name (e.g. map_n, sized_map, ...)
• Overload with enable_if differentiating whether the third argument is an iterator.

Avoid explicit mention to incclude directories in source path

It is strange to have things like

#include "include/common/common.h"

This cane be removed if we add the following to global CMake

include_directories("${CMAKE_SOURCE_DIR}/include")

Remove enable_flags header

Remove this file and the corresponding CMake machinery.

Users should compile with -DGRPPI_OMP or -DGRPPI_TBB and so

Integrate testing with CMake infrastructure

Testing should be availabe from CMake and not from separate scripts.

Proposed name change for thread policy

The policy parallel_execution_thr has a confusing name.

Is it thread? Or is it thrust?

I propose naming it parallel_execution_native as it is not delegating into another back-end, but implementing the execution itself.

Revise README.md

Revise README and add install notes.

Include ordering in header files

Revise ordering in all headers.

Order needs to be:

1. Including other files from GRPPI.
2. Including files from third party libraries.
3. Including files from the standard library.

In essens from most particular/local to most general.

Execution policies by const&

Revise all algorithms to take the execution policies by const&

Clean topmost CMake file

Remove all unneeded contents and simplify.

Define data mappings

A data mapping allows to access a set of values from some container.

The simplest mapping is a grppi::range_mapping which could be represented by two iterators to the same container. We may envision other mappings.

A grppi::range_mapping should be constructible from a container.

Possible sketch:

template <typename C>
class range_mapping {
public:
  range_mapping(); // Empty range. Do we need that one?
  range_mapping(C & c); // Range from container
  range_mapping(C::iterator f, C::iterator l) // Two iterators from a container

private:
  C::iterator begin_, end_;
};

Add an execution policy for thrust

Add an execution policy that can be used for the non-functional aspects of Thrust/CUDA

Install dirs in cmake

When I do maake install there are a number of issues:

• Old dependency from fastflow. It tries to install fastflow files.
• Include files are installed in /usr/local/GrPPI/include. The correct place would be /usr/local/include/grppi