Skip to content

Commit

Permalink
Update pre-commit hooks and add other checks
Browse files Browse the repository at this point in the history
  • Loading branch information
alazzaro committed Feb 15, 2022
1 parent 8a25a60 commit cc9e157
Show file tree
Hide file tree
Showing 18 changed files with 268 additions and 86 deletions.
178 changes: 178 additions & 0 deletions .clang-format
Original file line number Diff line number Diff line change
@@ -0,0 +1,178 @@
---
Language: Cpp
# BasedOnStyle: LLVM
AccessModifierOffset: -2
AlignAfterOpenBracket: Align
AlignArrayOfStructures: None
AlignConsecutiveMacros: None
AlignConsecutiveAssignments: None
AlignConsecutiveBitFields: None
AlignConsecutiveDeclarations: None
AlignEscapedNewlines: Right
AlignOperands: Align
AlignTrailingComments: true
AllowAllArgumentsOnNextLine: true
AllowAllConstructorInitializersOnNextLine: true
AllowAllParametersOfDeclarationOnNextLine: true
AllowShortEnumsOnASingleLine: true
AllowShortBlocksOnASingleLine: Never
AllowShortCaseLabelsOnASingleLine: false
AllowShortFunctionsOnASingleLine: All
AllowShortLambdasOnASingleLine: All
AllowShortIfStatementsOnASingleLine: Never
AllowShortLoopsOnASingleLine: false
AlwaysBreakAfterDefinitionReturnType: None
AlwaysBreakAfterReturnType: None
AlwaysBreakBeforeMultilineStrings: false
AlwaysBreakTemplateDeclarations: MultiLine
AttributeMacros:
- __capability
BinPackArguments: true
BinPackParameters: true
BraceWrapping:
AfterCaseLabel: false
AfterClass: false
AfterControlStatement: Never
AfterEnum: false
AfterFunction: false
AfterNamespace: false
AfterObjCDeclaration: false
AfterStruct: false
AfterUnion: false
AfterExternBlock: false
BeforeCatch: false
BeforeElse: false
BeforeLambdaBody: false
BeforeWhile: false
IndentBraces: false
SplitEmptyFunction: true
SplitEmptyRecord: true
SplitEmptyNamespace: true
BreakBeforeBinaryOperators: None
BreakBeforeConceptDeclarations: true
BreakBeforeBraces: Attach
BreakBeforeInheritanceComma: false
BreakInheritanceList: BeforeColon
BreakBeforeTernaryOperators: true
BreakConstructorInitializersBeforeComma: false
BreakConstructorInitializers: BeforeColon
BreakAfterJavaFieldAnnotations: false
BreakStringLiterals: true
ColumnLimit: 80
CommentPragmas: '^ IWYU pragma:'
CompactNamespaces: false
ConstructorInitializerAllOnOneLineOrOnePerLine: false
ConstructorInitializerIndentWidth: 4
ContinuationIndentWidth: 4
Cpp11BracedListStyle: true
DeriveLineEnding: true
DerivePointerAlignment: false
DisableFormat: false
EmptyLineAfterAccessModifier: Never
EmptyLineBeforeAccessModifier: LogicalBlock
ExperimentalAutoDetectBinPacking: false
FixNamespaceComments: true
ForEachMacros:
- foreach
- Q_FOREACH
- BOOST_FOREACH
IfMacros:
- KJ_IF_MAYBE
IncludeBlocks: Preserve
IncludeCategories:
- Regex: '^"(llvm|llvm-c|clang|clang-c)/'
Priority: 2
SortPriority: 0
CaseSensitive: false
- Regex: '^(<|"(gtest|gmock|isl|json)/)'
Priority: 3
SortPriority: 0
CaseSensitive: false
- Regex: '.*'
Priority: 1
SortPriority: 0
CaseSensitive: false
IncludeIsMainRegex: '(Test)?$'
IncludeIsMainSourceRegex: ''
IndentAccessModifiers: false
IndentCaseLabels: false
IndentCaseBlocks: false
IndentGotoLabels: true
IndentPPDirectives: None
IndentExternBlock: AfterExternBlock
IndentRequires: false
IndentWidth: 2
IndentWrappedFunctionNames: false
InsertTrailingCommas: None
JavaScriptQuotes: Leave
JavaScriptWrapImports: true
KeepEmptyLinesAtTheStartOfBlocks: true
LambdaBodyIndentation: Signature
MacroBlockBegin: ''
MacroBlockEnd: ''
MaxEmptyLinesToKeep: 1
NamespaceIndentation: None
ObjCBinPackProtocolList: Auto
ObjCBlockIndentWidth: 2
ObjCBreakBeforeNestedBlockParam: true
ObjCSpaceAfterProperty: false
ObjCSpaceBeforeProtocolList: true
PenaltyBreakAssignment: 2
PenaltyBreakBeforeFirstCallParameter: 19
PenaltyBreakComment: 300
PenaltyBreakFirstLessLess: 120
PenaltyBreakString: 1000
PenaltyBreakTemplateDeclaration: 10
PenaltyExcessCharacter: 1000000
PenaltyReturnTypeOnItsOwnLine: 60
PenaltyIndentedWhitespace: 0
PointerAlignment: Right
PPIndentWidth: -1
ReferenceAlignment: Pointer
ReflowComments: false
ShortNamespaceLines: 1
SortIncludes: CaseSensitive
SortJavaStaticImport: Before
SortUsingDeclarations: true
SpaceAfterCStyleCast: false
SpaceAfterLogicalNot: false
SpaceAfterTemplateKeyword: true
SpaceBeforeAssignmentOperators: true
SpaceBeforeCaseColon: false
SpaceBeforeCpp11BracedList: false
SpaceBeforeCtorInitializerColon: true
SpaceBeforeInheritanceColon: true
SpaceBeforeParens: ControlStatements
SpaceAroundPointerQualifiers: Default
SpaceBeforeRangeBasedForLoopColon: true
SpaceInEmptyBlock: false
SpaceInEmptyParentheses: false
SpacesBeforeTrailingComments: 1
SpacesInAngles: Never
SpacesInConditionalStatement: false
SpacesInContainerLiterals: true
SpacesInCStyleCastParentheses: false
SpacesInLineCommentPrefix:
Minimum: 1
Maximum: -1
SpacesInParentheses: false
SpacesInSquareBrackets: false
SpaceBeforeSquareBrackets: false
BitFieldColonSpacing: Both
Standard: Latest
StatementAttributeLikeMacros:
- Q_EMIT
StatementMacros:
- Q_UNUSED
- QT_REQUIRE_VERSION
TabWidth: 8
UseCRLF: false
UseTab: Never
WhitespaceSensitiveMacros:
- STRINGIZE
- PP_STRINGIZE
- BOOST_PP_STRINGIZE
- NS_SWIFT_NAME
- CF_SWIFT_NAME
...

10 changes: 7 additions & 3 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,26 @@ exclude: '^tools/(build_utils/fypp)'
fail_fast: false
repos:
- repo: https://github.com/ambv/black
rev: 21.5b1
rev: 22.1.0
hooks:
- id: black
name: Reformat Python files with the black code formatter
files: '^.*(/PACKAGE)|(\.py)$'
- repo: https://gitlab.com/pycqa/flake8
rev: 3.8.4
rev: 4.0.1
hooks:
- id: flake8
exclude: >-
(?x)^(
.cp2k/.*|
)$
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v3.4.0
rev: v4.1.0
hooks:
- id: check-ast
- id: check-yaml
- id: check-symlinks
- id: trailing-whitespace
- repo: https://github.com/pseewald/fprettify
rev: v0.3.7
hooks:
Expand Down
2 changes: 1 addition & 1 deletion LICENSE
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ convey the exclusion of warranty; and each file should have at least
the "copyright" line and a pointer to where the full notice is found.

DBCSR: Distributed Block Compressed Sparse Row matrix library
Copyright (C) by the DBCSR developers group - All rights reserved
Copyright (C) by the DBCSR developers group - All rights reserved

This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
Expand Down
2 changes: 1 addition & 1 deletion docs/guide/3-developer-guide/1-tooling/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ title: Tooling

# Build System

We support CMake for compilation. See [here](../../2-user-guide/1-installation/index.html) on how to compile and
We support CMake for compilation. See [here](../../2-user-guide/1-installation/index.html) on how to compile and
[here](../../2-user-guide/1-installation/1-cmake-build-recipes.html) for more CMake details.

Compilations is based on [Fypp](https://github.com/aradi/fypp) meta-progamming package, which is available as submodule.
Expand Down
2 changes: 1 addition & 1 deletion docs/guide/3-developer-guide/4-performance/1-insights.md
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ The columns describe:
- `AVERAGE`: averaged over all MPI ranks
- `MAXIMUM`: maximum over all MPI ranks
- `AVERAGE` and `MAXIMUM` can be used to locate load-imbalance or synchronization points.
- `MAXRANKS`:
- `MAXRANKS`:

#### Time spent in Just-In-Time (JIT) Compilation

Expand Down
2 changes: 1 addition & 1 deletion src/acc/libsmm_acc/predict/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ Explore the data interactively using the [provided Jupyter notebook](notebooks/i

#### 4. Train

For each algorithm, build a predictive model using decision trees and feature selection based on the features' permutation importance.
For each algorithm, build a predictive model using decision trees and feature selection based on the features' permutation importance.

```bash
./predict_train.py # --algo medium --folder /scratch/autotuning_dataset, e.g.
Expand Down
4 changes: 2 additions & 2 deletions src/acc/libsmm_acc/predict/predict_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def safe_pickle(data, file):
:param data: data to be pickled
:param file: file to pickle it into
"""
max_bytes = 2 ** 31 - 1 # Maximum number of bytes to write in one chunk
max_bytes = 2**31 - 1 # Maximum number of bytes to write in one chunk
pickle_out = pickle.dumps(data)
n_bytes = len(pickle_out)
with open(file, "wb") as f:
Expand All @@ -47,7 +47,7 @@ def safe_pickle_load(file_path):
:param data: data to be loaded through pickle
:param file: file to read from
"""
max_bytes = 2 ** 31 - 1 # Maximum number of bytes to read in one chunk
max_bytes = 2**31 - 1 # Maximum number of bytes to read in one chunk
bytes_in = bytearray(0)
input_size = os.path.getsize(file_path)
with open(file_path, "rb") as f:
Expand Down
6 changes: 3 additions & 3 deletions src/acc/libsmm_acc/predict/predict_train.py
Original file line number Diff line number Diff line change
Expand Up @@ -489,7 +489,7 @@ def read_data(algo, read_from, nrows, folder, log):
X = X.drop(cols_to_drop, axis=1)
log += print_and_log(
"X : {:>8,} x {:>8,} ({:>2.2} MB)".format(
len(X), len(X.columns), sys.getsizeof(X) / 10 ** 6
len(X), len(X.columns), sys.getsizeof(X) / 10**6
)
)
log += print_and_log("Head:")
Expand All @@ -505,7 +505,7 @@ def read_data(algo, read_from, nrows, folder, log):
log += print_and_log("\nRead Y")
Y = dd.read_parquet(parquet_data_file, columns=["perf_scaled"])
log += print_and_log(
"Y : {:>8,} ({:>2.2} MB)".format(len(Y), sys.getsizeof(Y) / 10 ** 6)
"Y : {:>8,} ({:>2.2} MB)".format(len(Y), sys.getsizeof(Y) / 10**6)
)
log += print_and_log("Head:")
log += print_and_log(Y.head())
Expand All @@ -516,7 +516,7 @@ def read_data(algo, read_from, nrows, folder, log):
X_mnk = dd.read_parquet(parquet_data_file, columns=["mnk"])
nrows_data = len(X_mnk.index)
log += print_and_log(
"X_mnk : {:>8,} ({:>2.2} MB)".format(nrows_data, sys.getsizeof(X_mnk) / 10 ** 6)
"X_mnk : {:>8,} ({:>2.2} MB)".format(nrows_data, sys.getsizeof(X_mnk) / 10**6)
)
log += print_and_log("Head:")
log += print_and_log(X_mnk.head())
Expand Down
2 changes: 1 addition & 1 deletion src/acc/libsmm_acc/tune/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ Each tune-directory contains a job file. Since there might be many tune-director
When `tune_submit.py` is called without arguments, it will just list the jobs that could be submitted:

```bash
$ ./tune_submit.py
$ ./tune_submit.py
tune_5x5x5: Would submit, run with "doit!"
tune_5x5x8: Would submit, run with "doit!"
tune_5x8x5: Would submit, run with "doit!"
Expand Down
18 changes: 9 additions & 9 deletions src/data/dbcsr.fypp
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,15 @@
#:set carry = 1
#:for i in range(0,len(num))
#:set outi = 0
#:if carry == 1
#:if carry == 1
#:if num[i] == 0
#:set outi = 1
#:set carry = 0
#:else
#:set outi = 0
#:set carry = 1
#:endif
#:else
#:else
#:set outi = num[i]
#:endif
#:mute
Expand All @@ -86,11 +86,11 @@ $: numout.append(outi)
#! generates a list of permutations from n entries
#! example n = 2 -> [[0,0],[0,1],[1,0],[1,1]] where 0/1 means present/not present
#:set idx = []
#:set newidx = []
#:set newidx = []
${init(idx,n)}$

#:set imax = pow(2,n)
#:for i in range(0,imax)
#:for i in range(0,imax)
$: permlist.append(idx)
${add_num(idx,newidx)}$
#:set idx = newidx
Expand All @@ -104,8 +104,8 @@ $: numout.append(outi)
#! generates permuted groups of variables from a variable list
#! optional variables that appear together may be grouped
#! example: varlist = [[var1], [var2,var3]]
#! this gives: vargroups = [ [[var1],[var2,var3]], [[var1]], [[var2,var3]], []]
#:set permlist = []
#! this gives: vargroups = [ [[var1],[var2,var3]], [[var1]], [[var2,var3]], []]
#:set permlist = []
${gen_permlist(permlist,len(varlist))}$
#:for p in permlist
#:set group = []
Expand Down Expand Up @@ -141,7 +141,7 @@ $: prefix * (bool(len(group))) + ", ".join([str(i) + ' = ' + str(i) for i in fla
#:def print_groupif(vargroups,varlist,i,check='PRESENT',prefix='')
#! for a group [[var1]] and a varlist [[var1]],[var2,var3]]
#! prints "(ELSE) IF (PRESENT(var1) .AND. .NOT. PRESENT(var2) .AND. .NOT. PRESENT(var3)) THEN"
#! to be used in a loop
#! to be used in a loop
#:set group = vargroups[i]
#:set diff = [item for item in varlist if item not in group]
#:set stat = "ELSE IF"
Expand All @@ -157,9 +157,9 @@ $: prefix * (bool(len(group))) + ", ".join([str(i) + ' = ' + str(i) for i in fla
${flatten(group,flatgroup)}$
${flatten(diff,flatdiff)}$
#:endmute
$: stat + "(" + " .AND. ".join([check + "(" + prefix + str(i) + ")" for i in flatgroup]) &
$: stat + "(" + " .AND. ".join([check + "(" + prefix + str(i) + ")" for i in flatgroup]) &
+ " .AND. " * (bool(len(diff)) * bool(len(diff) - len(varlist))) &
+ " .AND. ".join([".NOT. " + check + "(" + prefix + str(i) + ")" for i in flatdiff]) + ") THEN "
+ " .AND. ".join([".NOT. " + check + "(" + prefix + str(i) + ")" for i in flatdiff]) + ") THEN "
#:else
ELSE
#:endif
Expand Down
2 changes: 1 addition & 1 deletion src/dbcsr_api_c.F
Original file line number Diff line number Diff line change
Expand Up @@ -1406,7 +1406,7 @@ SUBROUTINE c_dbcsr_get_${var}$ (c_matrix, c_${var}$, c_size) BIND(C, name="c_dbc
END DO
#:else
DO i = 1, c_size
c_${var}$ (i) = ${var}$ (i);
c_${var}$ (i) = ${var}$ (i)
END DO
#:endif
NULLIFY (${var}$)
Expand Down
2 changes: 1 addition & 1 deletion src/ops/PACKAGE
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"description": "High level DBCSR operations",
"archive": "libdbcsr",
"requires": ["../acc", "../mpi", "../data", "../base", "../dist",
"requires": ["../acc", "../mpi", "../data", "../base", "../dist",
"../block", "../utils", "../core", "../mm", "../work"],
}
Loading

8 comments on commit cc9e157

@hfp
Copy link
Member

@hfp hfp commented on cc9e157 Feb 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would have been nice to ask before reformatting the C/C++ code. This format is particularly bad. For instance it broke single-line control flow but did not introduce curly braces which produced exactly a style considered dangerous.

@alazzaro
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm using clang-format... The problem is that we have FYPP directives in some of the files, which requires me to introduce a new script. There is a clang-format file if you want to configure it...

@alazzaro
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you give an example of format that you don't like? I see several options we can tune...

@hfp
Copy link
Member

@hfp hfp commented on cc9e157 Feb 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps this .clang-format is more suitable, but I already changed my code based on the current style to avoid the issues. Here are the changes I made to fix the issue: hfp@dce9237. I am currently CI-testing the code and will send a PR soon. I also think the LOC count went up a lot (for my code at least).

@alazzaro
Copy link
Member Author

@alazzaro alazzaro commented on cc9e157 Feb 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the .clang-format you are linking is very reasonable, so we can change the parameters in the current one. Should I or will you do that? @hfp

------- Update after private email exchange ---
You will do it. ;)

@hfp
Copy link
Member

@hfp hfp commented on cc9e157 Feb 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will do. What do you think about just replacing it? My clang-format perhaps does not repeat some defaults (hence it's smaller).

@hfp
Copy link
Member

@hfp hfp commented on cc9e157 Feb 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where do I have to specify formatting .cl files?

@alazzaro
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will do. What do you think about just replacing it? My clang-format perhaps does not repeat some defaults (hence it's smaller).

Yes, sure, I simply did the dump of the llvm format and changed two values ColumnLimit and ReflowComments, which I see you are changing to. So, go ahead.

Where do I have to specify formatting .cl files?

this is

files: \.(c|cc|cxx|cpp|frag|glsl|h|hpp|hxx|ih|ispc|ipp|java|js|m|mm|proto|textproto|vert)$

Please sign in to comment.