Skip to content

Commit

Permalink
CI workflow for style check (#226)
Browse files Browse the repository at this point in the history
* scripts/stylize.sh: by default, checks if stylization is required. enforces the stylization by -f flag.

* code-style ci test. must fail now since some files needs stylization.

* minor fix for scripts/stylize.sh

* returns error only when not enforced.

* astyle option file

* ran stylization

* some manual edits.
  • Loading branch information
dreamer2368 authored Aug 10, 2023
1 parent 0edcb6f commit 740f38b
Show file tree
Hide file tree
Showing 14 changed files with 326 additions and 218 deletions.
1 change: 1 addition & 0 deletions .astylerc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
--max-code-length=80
19 changes: 19 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,25 @@ on:
jobs:
docker-image:
uses: ./.github/workflows/docker.yml
code-style:
runs-on: ubuntu-latest
needs: [docker-image]
container:
image: ghcr.io/llnl/librom/librom_env:latest
options: --user 1001 --privileged
steps:
- name: Cancel previous runs
uses: styfle/[email protected]
with:
access_token: ${{ github.token }}
- name: Check out libROM
uses: actions/checkout@v3
- name: Artistic Style version (for information)
run: astyle --version
- name: Check Stylization
run: |
cd ${GITHUB_WORKSPACE}/scripts
./stylize.sh astyle
linux:
runs-on: ubuntu-latest
needs: [docker-image]
Expand Down
5 changes: 3 additions & 2 deletions lib/algo/DMD.cpp
100644 → 100755
Original file line number Diff line number Diff line change
Expand Up @@ -547,8 +547,9 @@ DMD::projectInitialCondition(const Vector* init, double t_offset)
Matrix* d_phi_imaginary_squared_2 = d_phi_imaginary->transposeMult(d_phi_real);
*d_phi_imaginary_squared -= *d_phi_imaginary_squared_2;

double* inverse_input = new double[d_phi_real_squared->numRows() *
d_phi_real_squared->numColumns() * 2];
const int dprs_row = d_phi_real_squared->numRows();
const int dprs_col = d_phi_real_squared->numColumns();
double* inverse_input = new double[dprs_row * dprs_col * 2];
for (int i = 0; i < d_phi_real_squared->numRows(); i++)
{
int k = 0;
Expand Down
34 changes: 23 additions & 11 deletions regression_tests/basisComparator.cpp
100644 → 100755
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@

using namespace std;

void compareBasis(string &baselineFile, string &targetFile, double errorBound, int numProcessors) {
void compareBasis(string &baselineFile, string &targetFile, double errorBound,
int numProcessors) {

MPI_Init(NULL, NULL);
// Get the number of processes
Expand All @@ -35,8 +36,10 @@ void compareBasis(string &baselineFile, string &targetFile, double errorBound, i
vector<double> reducedDiffVecNormL2;

CAROM::BasisReader baselineReader(baselineFile);
CAROM::Matrix *baselineBasis = (CAROM::Matrix*) baselineReader.getSpatialBasis(0.0);
CAROM::Vector *baselineSV = (CAROM::Vector*) baselineReader.getSingularValues(0.0);
CAROM::Matrix *baselineBasis =
(CAROM::Matrix*) baselineReader.getSpatialBasis(0.0);
CAROM::Vector *baselineSV =
(CAROM::Vector*) baselineReader.getSingularValues(0.0);
CAROM::BasisReader targetReader(targetFile);
CAROM::Matrix *targetBasis = (CAROM::Matrix*) targetReader.getSpatialBasis(0.0);
CAROM::BasisReader diffReader(baselineFile);
Expand All @@ -51,18 +54,21 @@ void compareBasis(string &baselineFile, string &targetFile, double errorBound, i
// Test basis matrices have the same dimensions
if (baselineNumRows != targetNumRows) {
cerr << "The number of rows of the two basis matrices \
are not equal in the following files: " << baselineFile << " and " << targetFile << endl;
are not equal in the following files: " << baselineFile
<< " and " << targetFile << endl;
MPI_Abort(MPI_COMM_WORLD, 1);
}
if (baselineNumColumns != targetNumColumns) {
cerr << "The number of columns of the two basis matrices \
are not equal in the following file: " << baselineFile << " and " << targetFile << endl;
are not equal in the following file: " << baselineFile
<< " and " << targetFile << endl;
MPI_Abort(MPI_COMM_WORLD, 1);
}
if (baselineSV->dim() != baselineNumColumns)
{
cerr << "The number of singular values does not equal the \
number of basis vectors in the following file: " << baselineFile << endl;
number of basis vectors in the following file: " << baselineFile
<< endl;
MPI_Abort(MPI_COMM_WORLD, 1);
}

Expand All @@ -76,7 +82,8 @@ number of basis vectors in the following file: " << baselineFile << endl;
}
catch (const exception& e) {
cerr << "Something went wrong when calculating the difference \
between the basis matrices in the following files: " << baselineFile << " and " << targetFile << endl;
between the basis matrices in the following files: " << baselineFile
<< " and " << targetFile << endl;
MPI_Abort(MPI_COMM_WORLD, 1);
}

Expand All @@ -88,8 +95,10 @@ between the basis matrices in the following files: " << baselineFile << " and "
}
}

MPI_Reduce(vecNormL2.data(), reducedVecNormL2.data(), baselineNumColumns, MPI_DOUBLE, MPI_SUM, 0, MPI_COMM_WORLD);
MPI_Reduce(diffVecNormL2.data(), reducedDiffVecNormL2.data(), baselineNumColumns, MPI_DOUBLE, MPI_SUM, 0, MPI_COMM_WORLD);
MPI_Reduce(vecNormL2.data(), reducedVecNormL2.data(), baselineNumColumns,
MPI_DOUBLE, MPI_SUM, 0, MPI_COMM_WORLD);
MPI_Reduce(diffVecNormL2.data(), reducedDiffVecNormL2.data(),
baselineNumColumns, MPI_DOUBLE, MPI_SUM, 0, MPI_COMM_WORLD);

if (rank == 0) {
double baselineNormL2 = 0.0;
Expand All @@ -113,9 +122,12 @@ between the basis matrices in the following files: " << baselineFile << " and "

// Test whether l2 norm is smaller than error bound
if (error > errorBound) {
cerr << "baselineNormL2 = " << baselineNormL2 << ", diffNormL2 = " << diffNormL2 << endl;
cerr << "baselineNormL2 = " << baselineNormL2 << ", diffNormL2 = " << diffNormL2
<< endl;
cerr << "error = " << error << endl;
cerr << "Error bound: " << errorBound << " was surpassed for the l2 norm of the difference of the basis matrices." << endl;
cerr << "Error bound: " << errorBound <<
" was surpassed for the l2 norm of the difference of the basis matrices." <<
endl;
MPI_Abort(MPI_COMM_WORLD, 1);
}
}
Expand Down
12 changes: 8 additions & 4 deletions regression_tests/fileComparator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@

using namespace std;

void compareFiles(ifstream &baselineFile, ifstream &targetFile, double errorBound) {
void compareFiles(ifstream &baselineFile, ifstream &targetFile,
double errorBound) {
string baselineLine, targetLine;
double baselineNum, targetNum;
int fileLine = 1;
Expand All @@ -27,7 +28,8 @@ void compareFiles(ifstream &baselineFile, ifstream &targetFile, double errorBoun
getline(baselineFile, baselineLine);
getline(targetFile, targetLine);
if (baselineLine == "" || targetLine == "") {
assert(baselineLine == targetLine || !(cerr << "The files are not the same length." << endl));
assert(baselineLine == targetLine
|| !(cerr << "The files are not the same length." << endl));
break;
}

Expand All @@ -50,12 +52,14 @@ void compareFiles(ifstream &baselineFile, ifstream &targetFile, double errorBoun
if (error > errorBound) {
cerr << "baseline = " << baselineNum << ", diff = " << diff << endl;
cerr << "error = " << error << endl;
cerr << "Error bound: " << errorBound << " was surpassed on line: " << fileLine << endl;
cerr << "Error bound: " << errorBound << " was surpassed on line: " << fileLine
<< endl;
abort();
}
fileLine++;
}
assert(targetFile.eof() || !(cerr << "The files are not the same length." << endl));
assert(targetFile.eof()
|| !(cerr << "The files are not the same length." << endl));
}

int main(int argc, char *argv[]) {
Expand Down
28 changes: 17 additions & 11 deletions regression_tests/solutionComparator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ int getDimensions(string &filePath) {
return count;
}

void compareSolutions(string &baselineFile, string &targetFile, double errorBound, int numProcessors) {
void compareSolutions(string &baselineFile, string &targetFile,
double errorBound, int numProcessors) {
int* baselineDim = new int[numProcessors];
istream** baselineFiles = new istream*[numProcessors];
int* targetDim = new int[numProcessors];
Expand All @@ -45,7 +46,8 @@ void compareSolutions(string &baselineFile, string &targetFile, double errorBoun
baselineDim[i] = getDimensions(baselineFile);
}
else {
cerr << "Something went wrong with opening the following file. Most likely it doesn't exist: " << baselineFile << endl;
cerr << "Something went wrong with opening the following file. Most likely it doesn't exist: "
<< baselineFile << endl;
abort();
}
cout << "Solution Comparator is Opening file: " << targetFile << endl;
Expand All @@ -54,7 +56,8 @@ void compareSolutions(string &baselineFile, string &targetFile, double errorBoun
targetDim[i] = getDimensions(targetFile);
}
else {
cerr << "Something went wrong with opening the following file. Most likely it doesn't exist: " << targetFile << endl;
cerr << "Something went wrong with opening the following file. Most likely it doesn't exist: "
<< targetFile << endl;
abort();
}
}
Expand All @@ -65,7 +68,8 @@ void compareSolutions(string &baselineFile, string &targetFile, double errorBoun

if (baseline.Size() != target.Size()) {
cerr << "The solution vectors are different dimensions." << endl;
cerr << "Baseline dim: " << baseline.Size() << ", Target dim: " << target.Size() << endl;
cerr << "Baseline dim: " << baseline.Size() << ", Target dim: " << target.Size()
<< endl;
abort();
}

Expand All @@ -81,14 +85,14 @@ between the solution vectors." << endl;
double baselineNormL2 = baseline.Norml2();
double diffNormL2 = diff.Norml2();
// Check for NaN
if(std::isnan(baselineNormL2)){
if(std::isnan(baselineNormL2)) {
std::cerr << "baselineNormL2 is NaN" << std::endl;
if(std::isnan(diffNormL2)){
if(std::isnan(diffNormL2)) {
std::cerr << "diffNormL2 is NaN" << std::endl;
}
abort();
}
if(std::isnan(diffNormL2)){
if(std::isnan(diffNormL2)) {
std::cerr << "diffNormL2 is NaN" << std::endl;
abort();
}
Expand All @@ -101,22 +105,24 @@ between the solution vectors." << endl;
}


if(std::isnan(baselineNormL2)){
if(std::isnan(baselineNormL2)) {
std::cout << "baselineNormL2 is NaN" << std::endl;
abort();
}

if(std::isnan(diffNormL2)){
if(std::isnan(diffNormL2)) {
std::cout << "diffNormL2 is NaN" << std::endl;
abort();
}


// Test whether l2 norm is smaller than error bound
if (error > errorBound) {
cerr << "baselineNormL2 = " << baselineNormL2 << ", diffNormL2 = " << diffNormL2 << endl;
cerr << "baselineNormL2 = " << baselineNormL2 << ", diffNormL2 = " << diffNormL2
<< endl;
cerr << "error = " << error << endl;
cerr << "Error bound: " << errorBound << " was surpassed for the l2 norm of the difference of the solutions." << endl;
cerr << "Error bound: " << errorBound <<
" was surpassed for the l2 norm of the difference of the solutions." << endl;
abort();
}
}
Expand Down
76 changes: 65 additions & 11 deletions scripts/stylize.sh
Original file line number Diff line number Diff line change
@@ -1,16 +1,70 @@
if [ "$1" == "" ] || [ $# -gt 1 ]; then
echo "Usage: ./stylize.sh /path/to/astyle/executable"
exit 0
#!/bin/bash
if [ "$1" == "" ] || [ $# -gt 2 ]; then
echo "Usage: ./stylize.sh [-f] /path/to/astyle/executable"
echo "Checks if stylization is required."
echo " -f: enforce stylization. By default, only the check is executed."
exit 1
fi
ASTYLE_BIN=$1
enforce=false

# parse the flags.
while getopts f: flag
do
case "${flag}" in
f)
enforce=true
ASTYLE_BIN=$2
;;
*)
echo "Unknown option."
exit 1
;;
esac
done

# astyle version check
ASTYLE_VER="Artistic Style Version 3.1"
astyle_version="$($ASTYLE_BIN --version)"
if [ "$astyle_version" != "$ASTYLE_VER" ]; then
printf "%s\n" "Invalid astyle version: '$astyle_version'"\
"Please use: '$ASTYLE_VER'"
exit 1
fi

# astyle dry-run if not enforced.
if [ $enforce != true ]; then
ASTYLE_BIN="$ASTYLE_BIN --dry-run"
fi

DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
DIR="$(dirname "$DIR")"

$1 --recursive --max-code-length=80 "$DIR/lib/*.cpp"
$1 --recursive --max-code-length=80 "$DIR/lib/*.h"
$1 --recursive --max-code-length=80 "$DIR/lib/*.hpp"
$1 --recursive --max-code-length=80 "$DIR/lib/*.c"
$1 --recursive --max-code-length=80 "$DIR/lib/*h.in"
$1 --recursive --max-code-length=80 "$DIR/tests/*.cpp"
$1 --recursive --max-code-length=80 "$DIR/examples/*.cpp"
$1 --recursive --max-code-length=80 "$DIR/examples/*.hpp"
FILELIST=("lib/*.cpp"
"lib/*.h"
"lib/*.hpp"
"lib/*.c"
"lib/*h.in"
"regression_tests/*.cpp"
"unit_tests/*.cpp"
"examples/*.cpp"
"examples/*.hpp")

ASTYLE_COMMAND="$ASTYLE_BIN --recursive --project=../.astylerc"

result=false
for files in ${FILELIST[@]}
do
echo $files
if $ASTYLE_COMMAND "$DIR/$files" | grep "Formatted"; then
result=true
fi
done

if [ $enforce != true ] && [ $result = true ]; then
echo "Files need stylization!"
echo "Please run stylization before merging the pull-request."
echo " 1. Install $ASTYLE_VER"
echo " 2. cd scripts && ./stylize.sh -f /path/to/astyle"
exit 1
fi
Loading

0 comments on commit 740f38b

Please sign in to comment.