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.

Sign up for GitHub

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

Jump to bottom

Code cleanup #3699

Merged
merged 18 commits into from
May 1, 2020
Merged

Code cleanup #3699

merged 18 commits into from
May 1, 2020

Conversation

jngrad
Copy link
Member

@jngrad jngrad commented May 1, 2020

Description of changes:

  • simplify handling of UBSAN and ASAN environment variables in CI and CMake
  • better explain GPU-related CMake options and variables
  • cleanup header includes
  • cleanup documentation
  • delete unused code, specfile and old UBSAN suppression file

jngrad added 17 commits May 1, 2020 01:52
@jngrad
Remove unused specfile
f085611 
A custom specfile is used when packaging espressomd on Fedora.
@jngrad
Refactor sanitizer environment variables
8d02c3b 
Set sanitizer variables in unit tests. Reduce code duplication.
@jngrad
Use the correct UBSAN suppression file
d657271
@jngrad
Remove old UBSAN suppression file
2d276a6 
Superseded by maintainer/CI/ubsan.supp from 1d17066.
@jngrad
Export ASAN variable in pypresso and core tests
e819733
@jngrad
Remove sanitizer variables from CI jobs
1c5a31d 
They are already exported in pypresso and unit tests.
@jngrad
Spell check and grammar check
559db7e
@jngrad
CI: gather Travis-CI env variables in one file
e0e315f
@jngrad
CI: build CUDA code in Travis-CI
4775a30 
Although Travis-CI runs without GPU, building CUDA code helps in
finding regressions.
@jngrad
docs: improve instructions to build with GPU
a3732c0
@jngrad
core: fix issues found by Clang-Tidy 10
33926a1
@jngrad
CI: use new Clang image with llvm-symbolizer
d6d466a 
Debug builds with ASAN need llvm-symbolizer to generate full reports.
@jngrad
docs: fix issues found by Clang 10 -Wdocumentation
dc93b8e
@jngrad
core: remove obsolete comments
12dbba7
@jngrad
core: narrow and remove includes with IWYU 10
411d0e3 
Narrow cstdio/iostream/sstream to source files that use them.
Remove superfluous header includes (mostly system headers).
@jngrad
utils: remove unincluded header files
435f391
@jngrad
tests: fix edge case in checkpoint test
d82b49d 
When running Python tests locally multiple times, the checkpoint
load tests may succeed even if the corresponding checkpoint save
tests failed. Old checkpoint files are now garbage collected.
@jngrad jngrad requested a review from fweik May 1, 2020 15:53
@jngrad
Copy link
Member Author

jngrad commented May 1, 2020

  • several commits need to be cherry-picked in 4.1.3
  • @fweik there should be no overlap with your atom decomposition PR

@jngrad jngrad added this to the Espresso 4.1.3 milestone May 1, 2020
fweik
fweik previously approved these changes May 1, 2020
View reviewed changes
Copy link
Contributor

@fweik fweik left a comment

Choose a reason for hiding this comment

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

LGTM, I made two comments, where you can decided if you want to address them, but you can also merge as is.

@codecov
Copy link

codecov bot commented May 1, 2020 •
edited
Loading

Codecov Report

Merging #3699 into python will increase coverage by 0%.
The diff coverage is 20%.

Impacted file tree graph

@@          Coverage Diff           @@
##           python   #3699   +/-   ##
======================================
  Coverage      88%     88%           
======================================
  Files         541     541           
  Lines       24714   24712    -2     
======================================
  Hits        21760   21760           
+ Misses       2954    2952    -2
Impacted Files Coverage Δ
src/core/CellStructure.hpp 94% <ø> (ø)
src/core/RuntimeErrorCollector.cpp 82% <ø> (ø)
src/core/accumulators/Correlator.cpp 66% <ø> (ø)
src/core/accumulators/Correlator.hpp 90% <ø> (ø)
src/core/actor/DipolarBarnesHut.hpp 85% <0%> (+4%) ⬆️
src/core/actor/DipolarDirectSum.hpp 88% <0%> (ø)
src/core/bonded_interactions/quartic.hpp 93% <ø> (ø)
src/core/cluster_analysis/Cluster.hpp 100% <ø> (ø)
src/core/cluster_analysis/ClusterStructure.hpp 100% <ø> (ø)
src/core/communication.hpp 100% <ø> (ø)
... and 40 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 927a82d...09294e7. Read the comment docs.

@jngrad jngrad added the automerge Merge with kodiak label May 1, 2020
@kodiakhq kodiakhq bot merged commit 1c3ac94 into espressomd:python May 1, 2020
@jngrad jngrad deleted the code-cleanup-4.2.0-p1 branch January 18, 2022 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fweik fweik fweik approved these changes

Assignees
No one assigned
Labels
automerge Merge with kodiak Core DevOps Documentation
Projects
None yet
Milestone
Espresso 4.1.3
Development

Successfully merging this pull request may close these issues.
2 participants
@jngrad @fweik