Reduce usage of local_cells #2991
Conversation
Codecov Report
@@ Coverage Diff @@
## python #2991 +/- ##
========================================
+ Coverage 82% 82% +<1%
========================================
Files 525 522 -3
Lines 26806 25584 -1222
========================================
- Hits 22014 21058 -956
+ Misses 4792 4526 -266
Continue to review full report at Codecov.
|
|
Did you consult with @reinaual, I think he was also looking into this. |
|
no, I'm not yet into mind reading |
|
Yeah, better work organization/mind reading technique is on the agenda for when I'm back this fall. For now I'm afraid you have to talk to other people every now and then... |
|
actually I think there is no magic behind writing a comment to an issue, also when we assign issues to somebody |
| @@ -60,10 +61,11 @@ void init_forces_ghosts(); | |||
| * <li> Calculate long range interaction forces | |||
| * </ol> | |||
| */ | |||
| void force_calc(); | |||
| void force_calc(const ParticleRange &particles); | |||
There was a problem hiding this comment.
force_calc actually needs access to the cells and to the particle index. To be clear, it should take something like a CellStructure as a parameter, which hopefully at some point in the future will encapsulate everything.
There was a problem hiding this comment.
you mean, we should also extract the cell_structure global and the ghost_cells?
There was a problem hiding this comment.
You can either leave the globals, and refactor this latter, or put the {local, global}_cells, potentially as dummy methods that return the globals into CellStructure and pass that.
They should not passed next to each other, because you actually have to have the cells that belong to the cell structure that is passed, and the particles that belong to those cells.
|
|
||
| if (integ_switch != INTEG_METHOD_STEEPEST_DESCENT) { | ||
| #ifdef ROTATION | ||
| convert_initial_torques(); | ||
| convert_initial_torques(local_cells.particles()); |
There was a problem hiding this comment.
Since the ParticleRange is used multiple times in this function, you should put it into local variables (after the resort), and reuse it. The range actually has state (because it caches its size which can not be calculated in constant time), so this has performance benefits.
There was a problem hiding this comment.
what do you mean with "after the resort"?
There was a problem hiding this comment.
This function function calls the particle resorting. This can invalidate the particle ranges, so after the resort the ranges have to be updated.
| @@ -49,16 +49,17 @@ int n_rigidbonds = 0; | |||
|
|
|||
| /** Calculates the corrections required for each of the particle coordinates | |||
| according to the RATTLE algorithm. Invoked from \ref correct_pos_shake()*/ | |||
| void compute_pos_corr_vec(int *repeat_); | |||
| void compute_pos_corr_vec(int *repeat_, const ParticleRange &particles); | |||
There was a problem hiding this comment.
Same comment as for force_calc applies, also the range should be reused withing thre function.
| revert(p); | ||
| } | ||
|
|
||
| void correct_vel_shake() { | ||
| void correct_vel_shake(const ParticleRange &particles, |
There was a problem hiding this comment.
Same comment as for force_calc applies, also the range should be reused withing thre function.
|
handed over to @reinaual |
2939: Documentation of VV, NPT, Thermostats r=RudolfWeeber a=christophlohrmann Fixes #2680 Description of changes: - Added sphinx documentation for velocity verlet, NPT - Added/Updated sphinx documentation for Langevin, NPT, LB, DPD thermostat - Found some papers as reference PR Checklist ------------ - [ ] Tests? - [ ] Interface - [ ] Core - [x] Docs? 3006: Reduce usage of local cells r=fweik a=KaiSzuttor Fixes part of #2899 Description of changes: - replacement for #2991 3008: Removed from __future__ r=jngrad a=jrfinkbeiner Fixes #3004 3015: Update checkpointing docs r=jngrad a=RudolfWeeber Fixes #2808 Co-authored-by: Christoph Lohrmann <clohrmann@icp.uni-stuttgart.de> Co-authored-by: Jean-Noël Grad <jgrad@icp.uni-stuttgart.de> Co-authored-by: Kai Szuttor <kai@icp.uni-stuttgart.de> Co-authored-by: Alexander Reinauer <st144434@stud.uni-stuttgart.de> Co-authored-by: Jan Finkbeiner <st144298@stud.uni-stuttgart.de> Co-authored-by: Rudolf Weeber <weeber@icp.uni-stuttgart.de>
2939: Documentation of VV, NPT, Thermostats r=RudolfWeeber a=christophlohrmann Fixes #2680 Description of changes: - Added sphinx documentation for velocity verlet, NPT - Added/Updated sphinx documentation for Langevin, NPT, LB, DPD thermostat - Found some papers as reference PR Checklist ------------ - [ ] Tests? - [ ] Interface - [ ] Core - [x] Docs? 3006: Reduce usage of local cells r=fweik a=KaiSzuttor Fixes part of #2899 Description of changes: - replacement for #2991 Co-authored-by: Christoph Lohrmann <clohrmann@icp.uni-stuttgart.de> Co-authored-by: Jean-Noël Grad <jgrad@icp.uni-stuttgart.de> Co-authored-by: Kai Szuttor <kai@icp.uni-stuttgart.de> Co-authored-by: Alexander Reinauer <st144434@stud.uni-stuttgart.de>
Part of the solution to #2899