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

Max cut #3053

Merged
merged 30 commits into from
Aug 9, 2019
Merged

Max cut #3053

merged 30 commits into from
Aug 9, 2019

Conversation

fweik
Copy link
Contributor

@fweik fweik commented Aug 5, 2019

Description of changes:

  • Replaced some globals related to cutoff calculation
  • Store range limits in cell_structure
  • Consistently use INACTIVE_CUTOFF/-1. for cutoffs
    of disabled kernels.

Further refactoring is blocked by the fact that the short-range kernels
with different cutoffs are fused together, and by the unclear resposibility
for the verlet list. The performance hacks that mix everything together
also do not help.

@fweik fweik requested a review from RudolfWeeber August 5, 2019 15:03
@codecov
Copy link

codecov bot commented Aug 6, 2019

Codecov Report

Merging #3053 into python will decrease coverage by <1%.
The diff coverage is 98%.

Impacted file tree graph

@@          Coverage Diff           @@
##           python   #3053   +/-   ##
======================================
- Coverage      82%     82%   -1%     
======================================
  Files         529     529           
  Lines       26086   26072   -14     
======================================
- Hits        21445   21432   -13     
+ Misses       4641    4640    -1
Impacted Files Coverage Δ
src/core/electrostatics_magnetostatics/dipole.cpp 63% <ø> (ø) ⬆️
src/core/statistics.cpp 61% <ø> (ø) ⬆️
src/core/domain_decomposition.hpp 100% <ø> (ø) ⬆️
src/core/forces_inline.hpp 82% <ø> (-1%) ⬇️
src/core/cells.hpp 100% <ø> (ø) ⬆️
src/core/MpiCallbacks.hpp 97% <ø> (ø) ⬆️
...re/bonded_interactions/bonded_interaction_data.hpp 100% <ø> (ø) ⬆️
src/core/pressure.cpp 90% <100%> (-1%) ⬇️
...bonded_interactions/nonbonded_interaction_data.cpp 100% <100%> (+1%) ⬆️
src/core/short_range_loop.hpp 100% <100%> (ø) ⬆️
... and 12 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 74b63f5...19dadca. Read the comment docs.

@KaiSzuttor
Copy link
Member

we should probably deactivate this clang-tidy warning...

@fweik
Copy link
Contributor Author

fweik commented Aug 7, 2019

I think the point is valid.

@RudolfWeeber
Copy link
Contributor

LGTM, except one doxygen thing

@@ -125,7 +117,8 @@ extern int min_num_cells;

  •            cells_on_geometry_change.
    
  • @param grid Number of nodes in each spatial dimension.
    */
    -void dd_on_geometry_change(int flags, const Utils::Vector3i &grid);
    +void dd_on_geometry_change(int flags, const Utils::Vector3i &grid,
  •                       double range);
    

Please add doxygen for range

Copy link
Contributor

@RudolfWeeber RudolfWeeber left a comment

Choose a reason for hiding this comment

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

bors r+

@jngrad
Copy link
Member

jngrad commented Aug 9, 2019

@RudolfWeeber there is a merge conflict, bors will fail and block processing of any other PR in the queue

@bors
Copy link
Contributor

bors bot commented Aug 9, 2019

Canceled

@jngrad jngrad self-assigned this Aug 9, 2019
@jngrad
Copy link
Member

jngrad commented Aug 9, 2019

bors retry

bors bot added a commit that referenced this pull request Aug 9, 2019
3053: Max cut r=RudolfWeeber a=fweik

Description of changes:
 - Replaced some globals related to cutoff calculation
 - Store range limits in cell_structure
 - Consistently use `INACTIVE_CUTOFF`/`-1.` for cutoffs
   of disabled kernels.

Further refactoring is blocked by the fact that the short-range kernels
with different cutoffs are fused together, and by the unclear resposibility
for the verlet list. The performance hacks that mix everything together
also do not help.

Co-authored-by: Florian Weik <[email protected]>
Co-authored-by: Jean-Noël Grad <[email protected]>
@bors
Copy link
Contributor

bors bot commented Aug 9, 2019

Build succeeded

@bors bors bot merged commit 19dadca into espressomd:python Aug 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants