eth-cscs / dla-future Goto Github PK

Gtest configuration variables should be markes as advanced.

Moreover by default:

• BUILD_GMOCK should be set to OFF
• BUILD_GTEST should be set to ON

Implement math utility ceilDiv and its test to check build system.

The wrapper object is templated on the type of the object it contains.
It is used to automatically set the value of the new future of a tile of the matrix
when the operation is completed.

It contains an object of type T and a promise of type hpx::promise<T>.
On destruction the promise is set with the value of the object.

see misc/example/m.h for reference.

Design and implement communicators.

The prototype interface is basic and should be redesigned.

To create the row and column communicators it is better to use MPI_Comm_split. See misc/code.(pdf|pptx)

I think it would be a nice thing to separate the role of communicating the data from preparing the data to be sent/received. The former one is addressed by #58, while this issue should address the latter one.

Data preparation happens on both sides of the communication:

• on the sender side, the memory has to be wrapped into an MPI Datatype;
• on the receiver side, the data value has to be extracted from the MPI Datatype;

The implementation would provide an API that maps between MPI and types used in the library, i.e. both primitive types and custom types that may require the definition of custom MPI Datatypes.

Types that should be managed are:

• primitive types (e.g. int, float, ...)
• contiguous array
• non-contiguous array

Contiguous and non-contiguous memory sequences of heterogeneous datatypes are not a requirement for now.

Add automation for the following tests:

• unit tests -> Jenkins
• coverage
• clang-format
• documentation build

Add src directory for the library (currently it is an header-only one), also allowing the separation between private and public interface.

• root
  • CMakeLists.txt
  • include (public headers)
  • src
    • include (private headers)
    • <source files>
    • CMakeLists.txt
  • test
    • ...

Templated on type and device (CPU / GPU)

Requirements:

• Construction from existing memory (m, n, pointer, leading dimension)
• Move constructor / move assignment
• Get pointer of (i, j) element
• Get value of (i, j) element (operator())
• Get size
• Get leading dimension

Currently clang-format format headers according to the coding style:

• if the main header is in a separated block (no way to fix it, but already mentioned in the coding style)
• if C and C++ standard headers are included using <>
• if third party libraries and DLAF headers are included with "".

The coding style do not mention any restriction on <> or ""

Question:
Should we restrict it? At what level?

IMHO we should restrict C and C++ to <> and DLAF to "", but I'm not sure about third party libs.

Add GPU support to CMake configuration

• Cuda
• CuBLAS

See misc/code.(pdf|pptx) and misc/example/m.h for details.

At the moment MKL Find CMake module looks for both LAPACK and SCALAPACK, but it would nice to just look for LAPACK.

I'm thinking of something like find_package(MKL REQUIRED COMPONENTS LAPACK) for just looking for LAPACK, and on the other side find_package(MKL REQUIRED COMPONENTS LAPACK SCALAPACK) or equivalently find_package(MKL REQUIRED COMPONENTS SCALAPACK), since LAPACK is a dependency for SCALAPACK.

See misc/code.(pdf|pptx) and misc/example/m.h for details.

MemoryChunk (and MemoryView) should allow an allocator to be used to allocate the memory.

However, the objects should not have a template dependency on the allocator type.

Add Doxygen to CMake configuration:

• Only if the build of the documentation is required.
• Find Doxygen
• Generate documentation for header and source files.

Part of the implementation of #57 is related to communication.

It would be nice to have a wrapper around MPI calls to ease the work and make the code more clean.

For instance, working with tiles, a way to easily broadcast a Tile would be useful.

* 8858566 (origin/api_message) Message API proposal
* dc65ad4 (origin/broadcast) fix mpi_datatype multiple inclusion
* d0018c4 rename parameters
* 6e65184 API proposal for broadcast
| * d10bf57 (origin/master) Fix gtest_hpx_main (#65)
| * ef1fd50 LayoutInfo and MatrixBase objects (#59)
| * 0d1d120 Tile improvements (#61)
|/
* 20a53ae (master) Introduce size_t arithmetic (#54)

This is the tree that make check_format fail.

The problem is that api_message branched from master and after that few files have been added to master. When check_format compares the trees, adds to the file to be checked also the ones newly added in master.

This happens because it compares not the master from which it branched, but the updated one. I think that this may create problem if I remove any file in my branch, since check_format could not find the file aynmore.

GTest source code is always fetched during CMake configure step, no matter if it is going to be used or not.

It may be a better choice to just fetch it in case of need, i.e. when DLAF_WITH_TEST=ON

The CI is failing on PizDaint due to a problem with CMake version.

The artifact output is:

ModuleCmd_Load.c(244):ERROR:105: Unable to locate a modulefile for 'java'
Currently Loaded Modulefiles:
  1) modules/3.2.11.3(default)
  2) craype-broadwell
  3) craype-network-aries
  4) craype/2.6.1(default)
  5) cray-mpich/7.7.10(default)
  6) slurm/19.05.3-2
  7) xalt/2.7.10
  8) gcc/8.3.0(default)
  9) pmi/5.0.14(default)
 10) atp/2.1.3(default)
 11) rca/2.2.20-7.0.1.1_4.9__g8e3fb5b.ari
 12) perftools-base/7.1.1(default)
 13) PrgEnv-gnu/6.0.5
 14) daint-mc
 15) CMake/.3.9.3
 16) intel/19.0.1.144
 17) EasyBuild/4.0.0
 18) EasyBuild-custom/cscs
 19) cdt/19.10
 20) CrayGNU/.19.10
 21) BLASPP/20190829-CrayGNU-19.10
 22) LAPACKPP/20190829-CrayGNU-19.10
 23) bzip2/.1.0.6-CrayGNU-19.10
 24) zlib/.1.2.11-CrayGNU-19.10
 25) cray-python/3.6.5.7
 26) Boost/1.70.0-CrayGNU-19.10-python3
 27) hwloc/.2.0.3
 28) jemalloc/.5.1.0-CrayGNU-19.10
 29) HPX/20190830-CrayGNU-19.10-jemalloc
+ DIR=build_Release
+ rm -rf build_Release
+ mkdir -p build_Release
+ cd build_Release
++ which srun
+ cmake -DCMAKE_BUILD_TYPE=Release -DDLAF_WITH_MKL=1 -DHPX_DIR=/apps/daint/SSL/rasolca/jenkins/daint-broadwell/software/HPX/20190830-CrayGNU-19.10-jemalloc -Dblaspp_DIR=/apps/daint/SSL/rasolca/jenkins/daint-broadwell/software/BLASPP/20190829-CrayGNU-19.10 -Dlapackpp_DIR=/apps/daint/SSL/rasolca/jenkins/daint-broadwell/software/LAPACKPP/20190829-CrayGNU-19.10 -DMPIEXEC_EXECUTABLE=/usr/bin/srun -DMPIEXEC_NUMPROC_FLAG=-n ../
CMake Error at CMakeLists.txt:11 (cmake_minimum_required):
  CMake 3.12.4 or higher is required.  You are running version 3.9.3


-- Configuring incomplete, errors occurred!

The variable NS3C_WITH_TEST is not a CMake CACHE variable, so the user can not set it.

We should start using compiler warnings.

Topics:

• What set of warnings do we want to use?
• Manage different compilers options
• Should warning be set just for internal build (PRIVATE) or also as a usage constraint (PUBLIC)?

If the format test fails the old output of the compilation stage is archived.

With #6 custom scripts have been introduced for blaspp and lapackpp, but looking into the packages, CMake config scripts are available.

Config scripts are preferred over FindModule scripts, moreover, official ones should be preferred over custom ones, since it does not seem that we are doing anything special to fit our needs.

In the source base there are comments but sometimes they are not rendered correctly or even not rendered at all due to wrong/lacking doxygen syntax.

Implement the first Linear Algebra algorithm of the library.

We have experience from on this, see the prototype code available in the private repository here for reference.

It will exploit the HPX library for implementing the task-based algorithm; it will work in-place so the input/output of the algorithm is a MatrixRW (#20).

Add GTest support to CMake configuration.

(See DLA-interface repository.)

Also set up the testing environment.

At the moment it is possible to add a test that uses HPX or MPI, but not both.

It would be useful for implementing tests for #58.

HPX tests which use gtest_hpx_main always return 0 even on case of failure.

Both blaspp and lapackpp are CMake projects, so they can be included as sub-projects and built together with the project.

I think that this option could ease the build of this project and, at the same time, it could help to avoid problems of using different libraries (i.e. BLAS, LAPACK).

DLAF_addTest should have a parameter to specify to which main function to link:

• NO -> custom main
• PLAIN -> main included in gtest
• HPX -> main starts and stops HPX
• MPI -> main initializes and finalizes MPI
• MPIHPX -> main initializes MPI and HPX

and others when needed.

It would be useful to have a folder containing quick start examples for some of the objects we have in the library.

It will be useful also for documentation purposes.

BLAS LAPACK wrappers for Tile objects:
templated on Type and Device.

It should checks that the matrix sizes are compatible.

required:

• gemm
• trsm

other operations as needed.

Interface should support Tile and Wrapper. Example for gemm:
gemm(transpose_a, transpose_b, T alpha, const Tile<T>& a, const Tile<T>& b, T beta, Tile<T>& c);
gemm(transpose_a, transpose_b, T alpha, const Wrapper<Tile<T>>& a, const Wrapper<Tile<T>>& b, T beta Wrapper<Tile<T>>&& c);

Configure the install/deploy step

• configure CMake install target
• set up for best interoperability with other CMake projects (modern way: export targets, ...)
• have an option for install tests

Memory size check for Tiles with either row or column size 0 is wrong.

BLAS / LAPACK installations may not provide header files.

This check should be disabled.

Add a test to check if the format of the sources (h, ipp, cpp) is correct. (i.e. clang-format was used)

If an header file is modified, make do not recompile the tests.

When a Matrix goes out of scope and gets destroyed, it does not wait for futures to be completed.

The output of MPI-based tests is not readable since there are multiple processes printing on the same shared console.

Add an output manager for DLAF_gtest_mpi_main.

Implement operator== and operator!= for Index2D and Size2D

See misc/code.(pdf|pptx) and misc/example/m.h for details.

Working with indexes often requires to perform arithmetic operations, mainly additions and multiplications, whose results have to be stored in std::size_t. For instance, this result can be used to refer memory positions for which an int is not enough.

Mixing different integer types in arithmetic operations may lead to wrong results, e.g. due to change of sign or due to overflow. Compiler warnings may help in discovering these kinds of problems, but in order to be able to work easily with them, it is useful to have a way to safely perform this basic operations.

In task-based algorithms, it would be nice to have a mechanism to "futurize" the communications.

Such a method would allow defining the order of operations in the tasks DAG, so that it would not allow executing communication operations in an order different from the one in which they are submitted.

The basic idea is to have an hpx::future<Communicator> so that HPX can take care of the scheduling. Moreover, it would be nice if this future becomes ready by itself, as soon as the communication is completed.

Basic working configuration which include:

• MPI
• HPX
• BLAS / LAPACK

Development of a distributed algorithm, like Cholesky factorization, may need to temporarily store data from other ranks. After usage, data are not useful anymore, but the algorithm may require other data in a next phase of the algorithm. With the term workspace, we refer to memory areas we use in our algorithms, but they are useful just for a limited phase of the computation. Often, working block-by-block, it comes out that workspaces are always of the same size (e.g. a Tile).

With the aim to avoid superfluous re-allocation of these memory areas, it would be nice to have a pool of pre-allocated workspaces, that can be acquired to be used in a particular phase of the algorithm. And then, as soon as they are not used anymore, they become available for the next usage.

Add C++ wrappers support to CMake configuration:

• BLASpp
• LAPACKpp

It is clear both from the template parameter name and from the documentation that IntType must be an integral type, but it is still possible to use it with floating types with wrong results (not truncated, and even if the user truncates it, it can lead to wrong values).

e.g. ceilDiv(3.0, 2.1)
the expected result is 2, but it gives ~1.4, that even truncated gives 1.

Define API and implement utilities for tile index.

E.g.

• element index to tile index + element index in the tile
• global tile index <-> local tile index

Suggestion:
define types for indexes to allow overload of functions (e.g. operator() may accept global or local indexes).