Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clang tidy #88

Merged
merged 9 commits into from May 31, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 6 additions & 6 deletions .clang-format
Expand Up @@ -2,7 +2,7 @@ Language: Cpp
BasedOnStyle: LLVM
# AccessModifierOffset: -2
# AlignAfterOpenBracket: Align
# AlignConsecutiveAssignments: false
AlignConsecutiveAssignments: true
# AlignConsecutiveDeclarations: false
# AlignEscapedNewlinesLeft: false
# AlignOperands: true
Expand All @@ -16,7 +16,7 @@ BasedOnStyle: LLVM
# AlwaysBreakAfterDefinitionReturnType: None
# AlwaysBreakAfterReturnType: None
# AlwaysBreakBeforeMultilineStrings: false
# AlwaysBreakTemplateDeclarations: false
AlwaysBreakTemplateDeclarations: true
BinPackArguments: false
BinPackParameters: false
# BraceWrapping:
Expand All @@ -31,10 +31,10 @@ BinPackParameters: false
# BeforeCatch: false
# BeforeElse: false
# IndentBraces: false
# BreakBeforeBinaryOperators: None
BreakBeforeBinaryOperators: All
# BreakBeforeBraces: Attach
# BreakBeforeTernaryOperators: true
# BreakConstructorInitializersBeforeComma: false
BreakConstructorInitializersBeforeComma: true
# BreakAfterJavaFieldAnnotations: false
# BreakStringLiterals: true
ColumnLimit: 120
Expand All @@ -53,7 +53,7 @@ IndentWidth: 4
# IndentWrappedFunctionNames: false
# JavaScriptQuotes: Leave
# JavaScriptWrapImports: true
# KeepEmptyLinesAtTheStartOfBlocks: true
KeepEmptyLinesAtTheStartOfBlocks: false
# MacroBlockBegin: ''
# MacroBlockEnd: ''
# MaxEmptyLinesToKeep: 1
Expand All @@ -71,7 +71,7 @@ IndentWidth: 4
# ReflowComments: true
SortIncludes: false
# SpaceAfterCStyleCast: false
# SpaceAfterTemplateKeyword: true
SpaceAfterTemplateKeyword: false
# SpaceBeforeAssignmentOperators: true
SpaceBeforeParens: Never
# SpaceInEmptyParentheses: false
Expand Down
8 changes: 6 additions & 2 deletions CMakeLists.txt
Expand Up @@ -64,7 +64,7 @@ endif()

if(CMAKE_VERSION VERSION_GREATER 3.6)
# Add clang-tidy if available
option(GOOFIT_TIDY_FIX "Perform fixes for Clang-Tidy (resets to OFF)" OFF)
option(GOOFIT_TIDY_FIX "Perform fixes for Clang-Tidy - changes source inplace" OFF)
find_program(
CLANG_TIDY_EXE
NAMES "clang-tidy"
Expand Down Expand Up @@ -305,10 +305,15 @@ endif()

# Adding simple libraries
add_subdirectory("extern/CLI11")
mark_as_advanced(CLI_EXAMPLES CLI_SINGLE_FILE CLI_SINGLE_FILE_TESTS CLI_TESTING)
add_subdirectory("extern/FeatureDetector")
set_target_properties(FeatureDetector PROPERTIES FOLDER extern)

## Format
add_subdirectory("extern/fmt")
set_target_properties(fmt PROPERTIES FOLDER extern)
mark_as_advanced(FMT_CPPFORMAT FMT_DOC FMT_INSTALL FMT_PEDANTIC FMT_TEST FMT_USE_CPP11)


add_library(rang INTERFACE)
target_include_directories(rang INTERFACE "${PROJECT_SOURCE_DIR}/extern/rang/include")
Expand Down Expand Up @@ -486,4 +491,3 @@ if(GOOFIT_TESTS)
add_subdirectory(tests)
endif()

set(GOOFIT_TIDY_FIX OFF CACHE BOOL "Perform fixes for Clang-Tidy (resets to OFF)" FORCE)
22 changes: 22 additions & 0 deletions CONTRIBUTING.md
Expand Up @@ -4,6 +4,27 @@

See [adding a package](doc/ADDING_EXTERN.md).

## C++

You should verify that the LLVM 4.0 based requirements pass. To fix the clang-tidy requirements:

```bash
cmake -DGOOFIT_TIDY_FIX=ON ..
make -j1
```

To run `clang-format` to correct source code spacing:

```bash
git ls-files -- '*.cu' '*.cc' '*.h' '*.cpp' | xargs clang-format -i -style=file
```

The modernize script should also not make any changes:

```bash
git ls-files -- '*.cu' '*.cc' '*.h' '*.cpp' | xargs ModernizeGoofit.py
```

## Git

GooFit is hosted as a [git](http://git-scm.com) repository on [GitHub](https://github.com).
Expand All @@ -27,6 +48,7 @@ There's no point in repeating all that info specifically for GooFit here, we do
If you don't have time to learn about git and pull requests, we still do want your contribution to GooFit. In that case please make an issue on GitHub and link to your updated files or patches e.g. in [gists](https://gist.github.com).
Note that this way you won't get credit for you contribution though in the commit history, so we prefer you make a GitHub pull request or ask for help in a GitHub issue on where you got stuck (fork -> clone -> branch -> edit -> commit -> push -> pull request) with making the pull request.


## Contact

Please use [Issues](https://github.com/GooFit/GooFit/issues) for suggestions,
Expand Down
2 changes: 1 addition & 1 deletion README.md
Expand Up @@ -87,7 +87,7 @@ Advanced Options:
* `-DGOOFIT_PYTHON=OFF`: Preliminary python bindings using [PyBind11].
* `-DGOOFIT_MAXPAR=1800`: The maximum number of parameters to allow. May cause memory issues if too large.
* You can enable sanitizers on non-CUDA builds with `-DSANITIZE_ADDRESS=ON`, `-DSANITIZE_MEMORY=ON`, `-DSANITIZE_THREAD=ON` or `-DSANITIZE_UNDEFINED=ON`.
* If `clang-tidy` is available, it will automatically be used to check the source. If you set `-DGOOFIT_TIDY_FIX=ON`, fixes will be applied to the GooFit source (This must be passed on command line, will not be cached).
* If `clang-tidy` is available, it will automatically be used to check the source. If you set `-DGOOFIT_TIDY_FIX=ON`, fixes will be applied to the GooFit source.

Note for targeting Tesla P100 or any `arch=6.0` device:
* Please use `-DGOOFIT_SEPARATE_COMP=ON` flags to compile.
Expand Down
4 changes: 2 additions & 2 deletions docs/SYSTEM_INSTALL.md
Expand Up @@ -34,7 +34,7 @@ source root-6/bin/thisroot.sh
A truly minimal system, Alpine gives you a working Docker system under 3 MB.

```bash
docker run -it alpine sh
docker run -it alpine
apk add --no-cache make cmake g++ git libexecinfo-dev
git clone --recursive https://github.com/GooFit/GooFit.git
cd GooFit
Expand All @@ -48,7 +48,7 @@ ctest
In the spirit of minimality, this is less instructive and contains more magic, but also would also work:

```bash
docker run -it alpine sh
docker run -it alpine
apk add --no-cache make cmake g++ git
git clone https://github.com/GooFit/GooFit.git
cd GooFit
Expand Down
70 changes: 37 additions & 33 deletions examples/2d_plot/2d_plot.cu
Expand Up @@ -18,16 +18,15 @@
using namespace std;
using namespace GooFit;

int main(int argc, char** argv) {

int main(int argc, char **argv) {
GooFit::Application app("2D plot example", argc, argv);

try {
app.run();
} catch (const GooFit::ParseError &e) {
} catch(const GooFit::ParseError &e) {
return app.exit(e);
}

// In real code, use a random device here
std::mt19937 gen(137);
std::normal_distribution<> dx(0.2, 1.1);
Expand All @@ -50,13 +49,16 @@ int main(int argc, char** argv) {
Variable yvar{"yvar", -5, 5};
UnbinnedDataSet data({&xvar, &yvar});

TH2F dataHist("dataHist", "",
xvar.getNumBins(), xvar.getLowerLimit(), xvar.getUpperLimit(),
yvar.getNumBins(), yvar.getLowerLimit(), yvar.getUpperLimit());
TH1F xvarHist("xvarHist", "",
xvar.getNumBins(), xvar.getLowerLimit(), xvar.getUpperLimit());
TH1F yvarHist("yvarHist", "",
yvar.getNumBins(), yvar.getLowerLimit(), yvar.getUpperLimit());
TH2F dataHist("dataHist",
"",
xvar.getNumBins(),
xvar.getLowerLimit(),
xvar.getUpperLimit(),
yvar.getNumBins(),
yvar.getLowerLimit(),
yvar.getUpperLimit());
TH1F xvarHist("xvarHist", "", xvar.getNumBins(), xvar.getLowerLimit(), xvar.getUpperLimit());
TH1F yvarHist("yvarHist", "", yvar.getNumBins(), yvar.getLowerLimit(), yvar.getUpperLimit());

dataHist.SetStats(false);
xvarHist.SetStats(false);
Expand All @@ -77,33 +79,36 @@ int main(int argc, char** argv) {
}
}

Variable xmean {"xmean", 0, 1, -10, 10};
Variable xsigm {"xsigm", 1, 0.5, 1.5};
Variable xmean{"xmean", 0, 1, -10, 10};
Variable xsigm{"xsigm", 1, 0.5, 1.5};
GaussianPdf xgauss("xgauss", &xvar, &xmean, &xsigm);

Variable ymean{"ymean", 0, 1, -10, 10};
Variable ysigm {"ysigm", 0.4, 0.1, 0.6};
Variable ysigm{"ysigm", 0.4, 0.1, 0.6};
GaussianPdf ygauss{"ygauss", &yvar, &ymean, &ysigm};

ProdPdf total("total", {&xgauss, &ygauss});
total.setData(&data);

FitManager fitter(&total);
fitter.fit();

TH2F pdfHist("pdfHist", "",
xvar.getNumBins(), xvar.getLowerLimit(), xvar.getUpperLimit(),
yvar.getNumBins(), yvar.getLowerLimit(), yvar.getUpperLimit());
TH1F xpdfHist("xpdfHist", "",
xvar.getNumBins(), xvar.getLowerLimit(), xvar.getUpperLimit());
TH1F ypdfHist("ypdfHist", "",
yvar.getNumBins(), yvar.getLowerLimit(), yvar.getUpperLimit());
TH2F pdfHist("pdfHist",
"",
xvar.getNumBins(),
xvar.getLowerLimit(),
xvar.getUpperLimit(),
yvar.getNumBins(),
yvar.getLowerLimit(),
yvar.getUpperLimit());
TH1F xpdfHist("xpdfHist", "", xvar.getNumBins(), xvar.getLowerLimit(), xvar.getUpperLimit());
TH1F ypdfHist("ypdfHist", "", yvar.getNumBins(), yvar.getLowerLimit(), yvar.getUpperLimit());

pdfHist.SetStats(false);
xpdfHist.SetStats(false);
ypdfHist.SetStats(false);

UnbinnedDataSet grid = total.makeGrid();
UnbinnedDataSet grid = total.makeGrid();
std::vector<std::vector<double>> pdfVals = total.getCompProbsAtDataPoints();

TCanvas foo;
Expand All @@ -121,23 +126,23 @@ int main(int argc, char** argv) {
}

for(int i = 0; i < xvar.getNumBins(); ++i) {
double val = xpdfHist.GetBinContent(i+1);
double val = xpdfHist.GetBinContent(i + 1);
val /= totalPdf;
val *= totalData;
xpdfHist.SetBinContent(i+1, val);
xpdfHist.SetBinContent(i + 1, val);
}

for(int i = 0; i < yvar.getNumBins(); ++i) {
double val = ypdfHist.GetBinContent(i+1);
double val = ypdfHist.GetBinContent(i + 1);
val /= totalPdf;
val *= totalData;
ypdfHist.SetBinContent(i+1, val);
ypdfHist.SetBinContent(i + 1, val);

for(int j = 0; j < xvar.getNumBins(); ++j) {
val = pdfHist.GetBinContent(j+1, i+1);
val = pdfHist.GetBinContent(j + 1, i + 1);
val /= totalPdf;
val *= totalData;
pdfHist.SetBinContent(j+1, i+1, val);
pdfHist.SetBinContent(j + 1, i + 1, val);
}
}

Expand All @@ -146,11 +151,11 @@ int main(int argc, char** argv) {

for(int i = 0; i < yvar.getNumBins(); ++i) {
for(int j = 0; j < xvar.getNumBins(); ++j) {
double pval = pdfHist.GetBinContent(j+1, i+1);
double dval = dataHist.GetBinContent(j+1, i+1);
double pval = pdfHist.GetBinContent(j + 1, i + 1);
double dval = dataHist.GetBinContent(j + 1, i + 1);
pval -= dval;
pval /= std::max(1.0, sqrt(dval));
pdfHist.SetBinContent(j+1, i+1, pval);
pdfHist.SetBinContent(j + 1, i + 1, pval);
}
}

Expand All @@ -174,6 +179,5 @@ int main(int argc, char** argv) {
ypdfHist.Draw("lsame");
foo.SaveAs("yhist.png");


return fitter;
}