Skip to content

Commit 66a6341

Browse files
P3M cleanup of ParticleRanges (#4841)
Continuation of #4721. Description of changes: - I continued on moving `ParticleRanges` out of function signatures and parsing `combined_ranges` instead. - For `charge_assign` I had to move the definition to the `.hpp` file to avoid specifying the type of these `boost` ranges. - I also removed the unused `kernel` method in `p3m.hpp` that probably was forgotten during some refactoring Moving the construction of the new boost ranges would require to change the signature of `long_range_kernel`, which would require to touch `long_range_forces`, which would require bigger refactoring. Through discussions with Rudolf the idea of an `P3MImpl` class to abstract implementation from the espresso implementation, as well as a base class that contains these ranges came up. I am not sure if this should be part of the PR, what do you think @jngrad?
2 parents cc3a319 + 7d6434b commit 66a6341

File tree

4 files changed

+62
-42
lines changed

4 files changed

+62
-42
lines changed

src/core/ParticlePropertyIterator.hpp

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,19 +42,25 @@ auto create_transform_range(ParticleRange const &particles, Kernel kernel) {
4242
}
4343
} // namespace detail
4444

45-
auto unfolded_pos_range(ParticleRange const &particles,
46-
BoxGeometry const &box) {
45+
inline auto unfolded_pos_range(ParticleRange const &particles,
46+
BoxGeometry const &box) {
4747
auto return_unfolded_pos = [&box](Particle &p) {
4848
return ::detail::unfolded_position(p.pos(), p.image_box(), box.length());
4949
};
5050
return detail::create_transform_range(particles, return_unfolded_pos);
5151
}
5252

53-
auto charge_range(ParticleRange const &particles) {
53+
inline auto pos_range(ParticleRange const &particles) {
54+
auto return_pos = [](Particle &p) -> Utils::Vector3d & { return p.pos(); };
55+
return detail::create_transform_range(particles, return_pos);
56+
}
57+
58+
inline auto charge_range(ParticleRange const &particles) {
5459
auto return_charge = [](Particle &p) -> double & { return p.q(); };
5560
return detail::create_transform_range(particles, return_charge);
5661
}
57-
auto force_range(ParticleRange const &particles) {
62+
63+
inline auto force_range(ParticleRange const &particles) {
5864
auto return_force = [](Particle &p) -> Utils::Vector3d & {
5965
return p.force();
6066
};

src/core/electrostatics/elc.cpp

Lines changed: 40 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232

3333
#include "BoxGeometry.hpp"
3434
#include "Particle.hpp"
35+
#include "ParticlePropertyIterator.hpp"
3536
#include "ParticleRange.hpp"
3637
#include "cell_system/CellStructure.hpp"
3738
#include "communication.hpp"
@@ -43,6 +44,7 @@
4344
#include <utils/math/sqr.hpp>
4445

4546
#include <boost/mpi/collectives/all_reduce.hpp>
47+
#include <boost/range/combine.hpp>
4648

4749
#include <algorithm>
4850
#include <cassert>
@@ -1118,59 +1120,62 @@ static void p3m_assign_image_charge(elc_data const &elc, CoulombP3M &p3m,
11181120
}
11191121
}
11201122

1121-
template <ChargeProtocol protocol>
1123+
template <ChargeProtocol protocol, typename combined_ranges>
11221124
void charge_assign(elc_data const &elc, CoulombP3M &solver,
1123-
ParticleRange const &particles) {
1125+
combined_ranges const &p_q_pos_range) {
11241126
if (protocol == ChargeProtocol::BOTH or protocol == ChargeProtocol::IMAGE) {
11251127
solver.p3m.inter_weights.reset(solver.p3m.params.cao);
11261128
}
11271129
/* prepare local FFT mesh */
11281130
for (int i = 0; i < solver.p3m.local_mesh.size; i++)
11291131
solver.p3m.rs_mesh[i] = 0.;
11301132

1131-
for (auto const &p : particles) {
1132-
if (p.q() != 0.) {
1133+
for (auto zipped : p_q_pos_range) {
1134+
auto const p_q = boost::get<0>(zipped);
1135+
auto const &p_pos = boost::get<1>(zipped);
1136+
if (p_q != 0.) {
11331137
if (protocol == ChargeProtocol::BOTH or
11341138
protocol == ChargeProtocol::REAL) {
1135-
solver.assign_charge(p.q(), p.pos(), solver.p3m.inter_weights);
1139+
solver.assign_charge(p_q, p_pos, solver.p3m.inter_weights);
11361140
}
11371141
if (protocol == ChargeProtocol::BOTH or
11381142
protocol == ChargeProtocol::IMAGE) {
1139-
p3m_assign_image_charge(elc, solver, p.q(), p.pos());
1143+
p3m_assign_image_charge(elc, solver, p_q, p_pos);
11401144
}
11411145
}
11421146
}
11431147
}
11441148

1145-
template <ChargeProtocol protocol>
1149+
template <ChargeProtocol protocol, typename combined_range>
11461150
void modify_p3m_sums(elc_data const &elc, CoulombP3M &solver,
1147-
ParticleRange const &particles) {
1151+
combined_range const &p_q_pos_range) {
11481152

11491153
Utils::Vector3d node_sums{};
1150-
for (auto const &p : particles) {
1151-
auto const q = p.q();
1152-
if (q != 0.) {
1153-
auto const z = p.pos()[2];
1154+
for (auto zipped : p_q_pos_range) {
1155+
auto const p_q = boost::get<0>(zipped);
1156+
auto const &p_pos = boost::get<1>(zipped);
1157+
if (p_q != 0.) {
1158+
auto const p_z = p_pos[2];
11541159

11551160
if (protocol == ChargeProtocol::BOTH or
11561161
protocol == ChargeProtocol::REAL) {
11571162
node_sums[0] += 1.;
1158-
node_sums[1] += Utils::sqr(q);
1159-
node_sums[2] += q;
1163+
node_sums[1] += Utils::sqr(p_q);
1164+
node_sums[2] += p_q;
11601165
}
11611166

11621167
if (protocol == ChargeProtocol::BOTH or
11631168
protocol == ChargeProtocol::IMAGE) {
1164-
if (z < elc.space_layer) {
1169+
if (p_z < elc.space_layer) {
11651170
node_sums[0] += 1.;
1166-
node_sums[1] += Utils::sqr(elc.delta_mid_bot * q);
1167-
node_sums[2] += elc.delta_mid_bot * q;
1171+
node_sums[1] += Utils::sqr(elc.delta_mid_bot * p_q);
1172+
node_sums[2] += elc.delta_mid_bot * p_q;
11681173
}
11691174

1170-
if (z > (elc.box_h - elc.space_layer)) {
1175+
if (p_z > (elc.box_h - elc.space_layer)) {
11711176
node_sums[0] += 1.;
1172-
node_sums[1] += Utils::sqr(elc.delta_mid_top * q);
1173-
node_sums[2] += elc.delta_mid_top * q;
1177+
node_sums[1] += Utils::sqr(elc.delta_mid_top * p_q);
1178+
node_sums[2] += elc.delta_mid_top * p_q;
11741179
}
11751180
}
11761181
}
@@ -1190,6 +1195,10 @@ double ElectrostaticLayerCorrection::long_range_energy(
11901195
auto &solver = *solver_ptr;
11911196
auto const &box_geo = *get_system().box_geo;
11921197

1198+
auto p_q_range = ParticlePropertyRange::charge_range(particles);
1199+
auto p_pos_range = ParticlePropertyRange::pos_range(particles);
1200+
auto p_q_pos_range = boost::combine(p_q_range, p_pos_range);
1201+
11931202
// assign the original charges (they may not have been assigned yet)
11941203
solver.charge_assign(particles);
11951204

@@ -1203,17 +1212,17 @@ double ElectrostaticLayerCorrection::long_range_energy(
12031212
0.5 * elc.dielectric_layers_self_energy(solver, box_geo, particles);
12041213

12051214
// assign both original and image charges
1206-
charge_assign<ChargeProtocol::BOTH>(elc, solver, particles);
1207-
modify_p3m_sums<ChargeProtocol::BOTH>(elc, solver, particles);
1215+
charge_assign<ChargeProtocol::BOTH>(elc, solver, p_q_pos_range);
1216+
modify_p3m_sums<ChargeProtocol::BOTH>(elc, solver, p_q_pos_range);
12081217
energy += 0.5 * solver.long_range_energy(particles);
12091218

12101219
// assign only the image charges now
1211-
charge_assign<ChargeProtocol::IMAGE>(elc, solver, particles);
1212-
modify_p3m_sums<ChargeProtocol::IMAGE>(elc, solver, particles);
1220+
charge_assign<ChargeProtocol::IMAGE>(elc, solver, p_q_pos_range);
1221+
modify_p3m_sums<ChargeProtocol::IMAGE>(elc, solver, p_q_pos_range);
12131222
energy -= 0.5 * solver.long_range_energy(particles);
12141223

12151224
// restore modified sums
1216-
modify_p3m_sums<ChargeProtocol::REAL>(elc, solver, particles);
1225+
modify_p3m_sums<ChargeProtocol::REAL>(elc, solver, p_q_pos_range);
12171226

12181227
return energy;
12191228
},
@@ -1226,17 +1235,20 @@ void ElectrostaticLayerCorrection::add_long_range_forces(
12261235
std::visit(
12271236
[this, &particles](auto const &solver_ptr) {
12281237
auto &solver = *solver_ptr;
1238+
auto p_q_range = ParticlePropertyRange::charge_range(particles);
1239+
auto p_pos_range = ParticlePropertyRange::pos_range(particles);
1240+
auto p_q_pos_range = boost::combine(p_q_range, p_pos_range);
12291241
if (elc.dielectric_contrast_on) {
12301242
auto const &box_geo = *get_system().box_geo;
1231-
modify_p3m_sums<ChargeProtocol::BOTH>(elc, solver, particles);
1232-
charge_assign<ChargeProtocol::BOTH>(elc, solver, particles);
1243+
modify_p3m_sums<ChargeProtocol::BOTH>(elc, solver, p_q_pos_range);
1244+
charge_assign<ChargeProtocol::BOTH>(elc, solver, p_q_pos_range);
12331245
elc.dielectric_layers_self_forces(solver, box_geo, particles);
12341246
} else {
12351247
solver.charge_assign(particles);
12361248
}
12371249
solver.add_long_range_forces(particles);
12381250
if (elc.dielectric_contrast_on) {
1239-
modify_p3m_sums<ChargeProtocol::REAL>(elc, solver, particles);
1251+
modify_p3m_sums<ChargeProtocol::REAL>(elc, solver, p_q_pos_range);
12401252
}
12411253
},
12421254
base_solver);

src/core/electrostatics/p3m.cpp

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -323,10 +323,13 @@ template <int cao> struct AssignCharge {
323323
[q, &p3m](int ind, double w) { p3m.rs_mesh[ind] += w * q; });
324324
}
325325

326-
void operator()(p3m_data_struct &p3m, ParticleRange const &particles) {
327-
for (auto &p : particles) {
328-
if (p.q() != 0.0) {
329-
this->operator()(p3m, p.q(), p.pos(), p3m.inter_weights);
326+
template <typename combined_ranges>
327+
void operator()(p3m_data_struct &p3m, combined_ranges const &p_q_pos_range) {
328+
for (auto zipped : p_q_pos_range) {
329+
auto const p_q = boost::get<0>(zipped);
330+
auto const &p_pos = boost::get<1>(zipped);
331+
if (p_q != 0.0) {
332+
this->operator()(p3m, p_q, p_pos, p3m.inter_weights);
330333
}
331334
}
332335
}
@@ -340,8 +343,11 @@ void CoulombP3M::charge_assign(ParticleRange const &particles) {
340343
for (int i = 0; i < p3m.local_mesh.size; i++)
341344
p3m.rs_mesh[i] = 0.0;
342345

343-
Utils::integral_parameter<int, AssignCharge, 1, 7>(p3m.params.cao, p3m,
344-
particles);
346+
auto p_q_range = ParticlePropertyRange::charge_range(particles);
347+
auto p_pos_range = ParticlePropertyRange::pos_range(particles);
348+
349+
Utils::integral_parameter<int, AssignCharge, 1, 7>(
350+
p3m.params.cao, p3m, boost::combine(p_q_range, p_pos_range));
345351
}
346352

347353
void CoulombP3M::assign_charge(double q, Utils::Vector3d const &real_pos,

src/core/electrostatics/p3m.hpp

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -99,10 +99,6 @@ struct CoulombP3M : public Coulomb::Actor<CoulombP3M> {
9999

100100
bool is_tuned() const { return m_is_tuned; }
101101

102-
/** Compute the k-space part of forces and energies. */
103-
double kernel(bool force_flag, bool energy_flag,
104-
ParticleRange const &particles);
105-
106102
/** @brief Recalculate all derived parameters. */
107103
void init();
108104
void on_activation() {

0 commit comments

Comments
 (0)