Skip to content

Commit

Permalink
Update code style guidelines.
Browse files Browse the repository at this point in the history
  • Loading branch information
Swirydowicz, Katarzyna authored and pelesh committed Sep 21, 2023
1 parent 2e0bfe3 commit eaa3752
Show file tree
Hide file tree
Showing 7 changed files with 162 additions and 79 deletions.
71 changes: 70 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,14 @@
### Error handling
Return values of member functions should be of type `int` and used for error handling. Functions return 0 if no error is encounter, return positive value for warnings and recoverable error, and negative value for irrecoverable errors.

### Output
If an output is needed (for example, a warning needs to be displayed), use `std::cout` and not `printf` as shown below. There should be a space before and after each `<<`.

```c++
std::cout << "index out of bounds. Row " << i << " starts at: " << start << " and ends at " << end << std::endl;
```


### Member variable naming

Member variable names should use C-style name format and end with trailing underscore `_`.
Expand All @@ -24,6 +32,31 @@ int another_function(); // No, using C-style name format
int YetAnotherFunction(); // No, using uppercase camel name format
```
### Class names
Class names should start with a capital letter. For instance, `Vector` and `Matrix` are valid class names, while `point` is not.
### Enums (enumerated types)
Always define `enum`s inside `ReSolve` namespace. Type names should be capitalized and the constant names should be uppercase with underscores (but there is no underscore at the end!).
```c++
enum ExampleEnum { CONST_ONE = 0,
CONST_TWO = 8,
YET_ANOTHER_CONST = 17 };
```

### Constants

If a constant is used in more than one file, define it in `Common.h`. Constants names should be capitalized.

```c++
constexpr double Pi = 3.1415; // No, it should be all caps
constexpr double SQRT_TWO = 1.4142 // No, there is an underscore
constexpr double SQRTTWO_ = 1.4142 // No, there is an underscore
constexpr double EXP = 2.7183 // Yes
```

### Pointers and references

The pointer `*` or reference `&` belong to the type and there should be no space between them and the type name.
Expand All @@ -37,6 +70,22 @@ int & n; // No, the reference symbol is a part of `int&` type
### Indentation
Use only spaces for indentation, not tabs. Indent size is 2 spaces.

When defining a class, the code blocks after `private`, `public` and `protected` should be indented. There should be an empty line before each definition (except the first one). See example below.
```c++
class SomeClass
{
public:
SomeClass();
~SomeClass();

private:
int some_variable_;

protected:
void someFunction();
};
```

### Braces
Namespaces, classes and functions: use new line afterwards, i.e.,
```c++
Expand All @@ -47,7 +96,7 @@ namespace someNamespace
```
For short functions (i.e., empty constructor), do not inline braces.
```c++
classA::classA()
ClassA::ClassA()
{
}
```
Expand All @@ -59,6 +108,13 @@ if (cond == true) {
// some other code
}
```
Have a space between keywords `for`, `while` and `if` and the parenthesis as shown here:
```c++
for (int i = 0; i < n; ++i) {
// some code
}
```

Do not use one-line `if`s and `for`s. Always use braces.

### Use of spaces and newlines
Expand Down Expand Up @@ -92,6 +148,19 @@ int main()
}
```

Also, leave one empty line between `system` includes and `resolve` includes, i.e.,
```c++
#include <cstring>

#include <resolve/matrix/Coo.hpp>

int main()
{
//some code
return 0;
}
```
The `system` includes should always be listed first.

### Using namespaces
All classes should be in namespace `ReSolve`. If needed, define additional namespaces inside `ReSolve`.
Expand Down
9 changes: 6 additions & 3 deletions resolve/Common.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,11 @@ namespace ReSolve {
using real_type = double;
using index_type = int;

constexpr real_type zero = 0.0;
constexpr real_type one = 1.0;
constexpr real_type minusone = -1.0;
namespace constants
{
constexpr real_type ZERO = 0.0;
constexpr real_type ONE = 1.0;
constexpr real_type MINUSONE = -1.0;
}

} // namespace ReSolve
25 changes: 13 additions & 12 deletions resolve/GramSchmidt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ namespace ReSolve
this->setup_complete_ = false;
}

GramSchmidt::GramSchmidt(VectorHandler* vh, GS_variant variant)
GramSchmidt::GramSchmidt(VectorHandler* vh, GSVariant variant)
{
this->setVariant(variant);
this->vector_handler_ = vh;
Expand Down Expand Up @@ -60,17 +60,17 @@ namespace ReSolve
}
}

int GramSchmidt::setVariant(GS_variant variant)
int GramSchmidt::setVariant(GSVariant variant)
{
if ((variant != mgs) && (variant != cgs2) && (variant != mgs_two_synch) && (variant != mgs_pm) && (variant != cgs)) {
if ((variant != mgs) && (variant != cgs2) && (variant != mgs_two_synch) && (variant != mgs_pm) && (variant != cgs1)) {
this->variant_ = mgs;
return 2;
}
variant_ = variant;
return 0;
}

GS_variant GramSchmidt::getVariant()
GSVariant GramSchmidt::getVariant()
{
return variant_;
}
Expand Down Expand Up @@ -114,6 +114,7 @@ namespace ReSolve
//this always happen on the GPU
int GramSchmidt::orthogonalize(index_type n, vector::Vector* V, real_type* H, index_type i, std::string memspace)
{
using namespace constants;

if (memspace == "cuda") { // or hip

Expand Down Expand Up @@ -141,28 +142,28 @@ namespace ReSolve
t = 1.0/t;
vector_handler_->scal(&t, vec_w_, "cuda");
} else {
assert(0 && "Gram-Schmidt failed, vector with zero norm\n");
assert(0 && "Gram-Schmidt failed, vector with ZERO norm\n");
return -1;
}
break;
case cgs2:

vec_v_->setData(V->getVectorData(i + 1, "cuda"), "cuda");
vector_handler_->gemv("T", n, i + 1, &one, &zero, V, vec_v_, vec_Hcolumn_,"cuda");
vector_handler_->gemv("T", n, i + 1, &ONE, &ZERO, V, vec_v_, vec_Hcolumn_,"cuda");

// V(:,i+1) = V(:, i+1) - V(:,1:i)*Hcol
vector_handler_->gemv("N", n, i + 1, &one, &minusone, V, vec_Hcolumn_, vec_v_, "cuda" );
vector_handler_->gemv("N", n, i + 1, &ONE, &MINUSONE, V, vec_Hcolumn_, vec_v_, "cuda" );

// copy H_col to aux, we will need it later
vec_Hcolumn_->setDataUpdated("cuda");
vec_Hcolumn_->setCurrentSize(i + 1);
vec_Hcolumn_->deepCopyVectorData(h_aux_, 0, "cpu");

//Hcol = V(:,1:i)^T*V(:,i+1);
vector_handler_->gemv("T", n, i + 1, &one, &zero, V, vec_v_, vec_Hcolumn_,"cuda");
vector_handler_->gemv("T", n, i + 1, &ONE, &ZERO, V, vec_v_, vec_Hcolumn_,"cuda");

// V(:,i+1) = V(:, i+1) - V(:,1:i)*Hcol
vector_handler_->gemv("N", n, i + 1, &one, &minusone, V, vec_Hcolumn_, vec_v_, "cuda" );
vector_handler_->gemv("N", n, i + 1, &ONE, &MINUSONE, V, vec_Hcolumn_, vec_v_, "cuda" );

// copy H_col to H
vec_Hcolumn_->setDataUpdated("cuda");
Expand All @@ -182,7 +183,7 @@ namespace ReSolve
t = 1.0/t;
vector_handler_->scal(&t, vec_v_, "cuda");
} else {
assert(0 && "Gram-Schmidt failed, vector with zero norm\n");
assert(0 && "Gram-Schmidt failed, vector with ZERO norm\n");
return -1;
}
return 0;
Expand Down Expand Up @@ -225,7 +226,7 @@ namespace ReSolve
t = 1.0 / t;
vector_handler_->scal(&t, vec_w_, "cuda");
} else {
assert(0 && "Iterative refinement failed, Krylov vector with zero norm\n");
assert(0 && "Iterative refinement failed, Krylov vector with ZERO norm\n");
return -1;
}
return 0;
Expand Down Expand Up @@ -295,7 +296,7 @@ namespace ReSolve
t = 1.0 / t;
vector_handler_->scal(&t, vec_w_, "cuda");
} else {
assert(0 && "Iterative refinement failed, Krylov vector with zero norm\n");
assert(0 && "Iterative refinement failed, Krylov vector with ZERO norm\n");
return -1;
}
return 0;
Expand Down
14 changes: 9 additions & 5 deletions resolve/GramSchmidt.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,24 +5,28 @@
#include <cassert>
namespace ReSolve
{
enum GS_variant { mgs, cgs2, mgs_two_synch, mgs_pm, cgs};
enum GSVariant { mgs = 0,
cgs2 = 1,
mgs_two_synch = 2,
mgs_pm = 3,
cgs1 = 4 };
class GramSchmidt
{
using vector_type = vector::Vector;
public:
GramSchmidt();
GramSchmidt(VectorHandler* vh, GS_variant variant);
GramSchmidt(VectorHandler* vh, GSVariant variant);
~GramSchmidt();
int setVariant(GS_variant variant);
GS_variant getVariant();
int setVariant(GSVariant variant);
GSVariant getVariant();
real_type* getL(); //only for low synch, returns null ptr otherwise

int setup(index_type n, index_type restart);
int orthogonalize(index_type n, vector_type* V, real_type* H, index_type i, std::string memspace);

private:

GS_variant variant_;
GSVariant variant_;
bool setup_complete_; //to avoid double allocations and stuff

index_type num_vecs_; //the same as restart
Expand Down
16 changes: 9 additions & 7 deletions resolve/LinSolverIterativeFGMRES.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@ namespace ReSolve

int LinSolverIterativeFGMRES::solve(vector_type* rhs, vector_type* x)
{
using namespace constants;

int outer_flag = 1;
int notconv = 1;
int i = 0;
Expand All @@ -108,7 +110,7 @@ namespace ReSolve
//V[0] = b-A*x_0

rhs->deepCopyVectorData(d_V_->getData("cuda"), 0, "cuda");
matrix_handler_->matvec(A_, x, d_V_, &minusone, &one, "csr", "cuda");
matrix_handler_->matvec(A_, x, d_V_, &MINUSONE, &ONE, "csr", "cuda");
rnorm = 0.0;
bnorm = vector_handler_->dot(rhs, rhs, "cuda");
rnorm = vector_handler_->dot(d_V_, d_V_, "cuda");
Expand All @@ -127,13 +129,13 @@ namespace ReSolve
}
int exit_cond = 0;
if (conv_cond_ == 0){
exit_cond = ((fabs(rnorm - zero) <= EPSILON));
exit_cond = ((fabs(rnorm - ZERO) <= EPSILON));
} else {
if (conv_cond_ == 1){
exit_cond = ((fabs(rnorm - zero) <= EPSILON) || (rnorm < tol_));
exit_cond = ((fabs(rnorm - ZERO) <= EPSILON) || (rnorm < tol_));
} else {
if (conv_cond_ == 2){
exit_cond = ((fabs(rnorm - zero) <= EPSILON) || (rnorm < (tol_*bnorm)));
exit_cond = ((fabs(rnorm - ZERO) <= EPSILON) || (rnorm < (tol_*bnorm)));
}
}
}
Expand Down Expand Up @@ -168,7 +170,7 @@ namespace ReSolve

vec_v->setData( d_V_->getVectorData(i + 1, "cuda"), "cuda");

matrix_handler_->matvec(A_, vec_z, vec_v, &one, &zero,"csr", "cuda");
matrix_handler_->matvec(A_, vec_z, vec_v, &ONE, &ZERO,"csr", "cuda");

// orthogonalize V[i+1], form a column of h_H_

Expand All @@ -186,7 +188,7 @@ namespace ReSolve
double Hii1 = h_H_[(i) * (restart_ + 1) + i + 1];
double gam = sqrt(Hii * Hii + Hii1 * Hii1);

if(fabs(gam - zero) <= EPSILON) {
if(fabs(gam - ZERO) <= EPSILON) {
gam = EPSMAC;
}

Expand Down Expand Up @@ -233,7 +235,7 @@ namespace ReSolve
}

rhs->deepCopyVectorData(d_V_->getData("cuda"), 0, "cuda");
matrix_handler_->matvec(A_, x, d_V_, &minusone, &one,"csr", "cuda");
matrix_handler_->matvec(A_, x, d_V_, &MINUSONE, &ONE,"csr", "cuda");
rnorm = vector_handler_->dot(d_V_, d_V_, "cuda");
// rnorm = ||V_1||
rnorm = sqrt(rnorm);
Expand Down
7 changes: 4 additions & 3 deletions resolve/matrix/MatrixHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ namespace ReSolve {
}
if (coo_rows[i] == coo_cols[i]){
if (diag_control[coo_rows[i]] > 0) {
//duplicate
//duplicate
nnz_unpacked_no_duplicates--;
nnz_no_duplicates--;
}
Expand Down Expand Up @@ -230,6 +230,7 @@ namespace ReSolve {
std::string matrixFormat,
std::string memspace)
{
using namespace constants;
int error_sum = 0;
if (matrixFormat == "csr"){
matrix::Csr* A = (matrix::Csr*) Ageneric;
Expand Down Expand Up @@ -272,10 +273,10 @@ namespace ReSolve {

status = cusparseSpMV_bufferSize(handle_cusparse,
CUSPARSE_OPERATION_NON_TRANSPOSE,
&minusone,
&MINUSONE,
matA,
vecx,
&one,
&ONE,
vecAx,
CUDA_R_64F,
CUSPARSE_SPMV_CSR_ALG2,
Expand Down
Loading

0 comments on commit eaa3752

Please sign in to comment.