Some improvement suggestions

[GiovaGa]

Reading through the document, I found the following unclear or possibly wrong things:

• On page 4, in exercise 3 the matrix A is defined but not used until after exercise 4 in the MatrixFileReader examples.
• On page 5, in the snippet grb::utils::MatrixFileReader< double > parser( in, true ); it is unclear of what type in should be.
• In section 3.4 (Numerical Linear Algebra), it would probably be much clearer to include arguments in the functions e.g. grb::dot( grb::Vector<T> u, grb::Vector<T> v ) instead of grb::dot
• grb::eWiseAdd and grb::eWiseMul are suggested in the guide, but are deprecated in the documentation. (this could be a separate issue)
• At the end of page 7 there is the sentence: "Different from grb::set, the grb::mxv is an in-place operation". The phrase is possibly unclear, as . It is probably more clear to remark that all operations are in-place unless otherwise stated (such as grb::set).
• When introducing grb::set, it is not clear that it does not allocate additional memory, but it just changes default values (so it can be used instead of grb::clear without performance penalties).
• On page 8 there is a grb:set (missing a second colon);
• On page 8, it is said that "the primitives explained above takes a mandatory semiring argument'. But in the documentation the semiring parameter has a default value.
• In exercise 8, point 2 is "use grb::set and grb::setElement to assign values (see below for a suggestion);". But as far as I understand, these functions cannot be used to initialize a matrix with multiple different values. For that other functions are needed.
• On page 9, it is said that "A semiring is a combination of a multiplicative operator and an additive monoid." For someone with a mathematical background this sentence seems just wrong. I think it is better to clarify that the additional properties (e.g. distributive property) are not always required in ALP.
• About grb::apply, on page 10 it is said that "The function grb::apply(z, x, op) applies a given unary functor op to each element of vector (or matrix) x". But it seems that the most important aspect of grb::apply is keeping track of data dependencies.
  Additionally in the middle of the paragraph there are examples of using grb::eWiseApply, but nowhere is it clarified the difference between the two (for example why they cannot be overloaded).
• On page 10, there is a grb::apply(z, x, grb::operators::add<double>(), 5.0). The order of arguments seems just wrong. It should be grb::apply(z, x, 5.0, grb::operators::add<double>() ).
• There are some wild c++17 in the installation/building instructions. I guess everything should be c++11

[djelovina]

On page 4, in exercise 3 the matrix A is defined but not used until after exercise 4 in the MatrixFileReader examples.

This is fine, the Matrix is a basic container and it should be introduced here.

On page 5, in the snippet grb::utils::MatrixFileReader< double > parser( in, true ); it is unclear of what type in should be.

Added more descriptive code

[djelovina]

grb::eWiseAdd and grb::eWiseMul are suggested in the guide, but are deprecated in the documentation. (this could be a separate issue)

@anyzelman are these deprecated in the documentation? I thought that grb::eWiseMulAdd is deprecated, but I am not sure about these two?

[djelovina]

At the end of page 7 there is the sentence: "Different from grb::set, the grb::mxv is an in-place operation". The phrase is possibly unclear, as . It is probably more clear to remark that all operations are in-place unless otherwise stated (such as grb::set).

changed this part

When introducing grb::set, it is not clear that it does not allocate additional memory, but it just changes default values (so it can be used instead of grb::clear without performance penalties).

This is clarified now

On page 8 there is a grb:set (missing a second colon);

fixed

On page 8, it is said that "the primitives explained above takes a mandatory semiring argument'. But in the documentation the semiring parameter has a default value.

This part is improved.

In exercise 8, point 2 is "use grb::set and grb::setElement to assign values (see below for a suggestion);". But as far as I understand, these functions cannot be used to initialize a matrix with multiple different values. For that other functions are needed.

grb::set currently supports vectors and it should support Matrix after the fix, grb::setElement supports vectors only, I add comment about this.

On page 9, it is said that "A semiring is a combination of a multiplicative operator and an additive monoid." For someone with a mathematical background this sentence seems just wrong. I think it is better to clarify that the additional properties (e.g. distributive property) are not always required in ALP.

This is rephrased

About grb::apply, on page 10 it is said that "The function grb::apply(z, x, op) applies a given unary functor op to each element of vector (or matrix) x". But it seems that the most important aspect of grb::apply is keeping track of data dependencies.
Additionally in the middle of the paragraph there are examples of using grb::eWiseApply, but nowhere is it clarified the difference between the two (for example why they cannot be overloaded).

This part was wrong and not clear, fixed now.

On page 10, there is a grb::apply(z, x, grb::operators::add<double>(), 5.0). The order of arguments seems just wrong. It should be grb::apply(z, x, 5.0, grb::operators::add<double>() ).

This is removed now.

There are some wild c++17 in the installation/building instructions. I guess everything should be c++11

There is already need to switch to C++14 for the dense backend. For C++17 it is probably still too early.