Skip to content

Commit

Permalink
Core maintenance (#4606)
Browse files Browse the repository at this point in the history
Description of changes:
- fix multiple compiler diagnostics
  • Loading branch information
kodiakhq[bot] authored Nov 18, 2022
2 parents 3638f20 + 6e9276d commit ff8a010
Show file tree
Hide file tree
Showing 27 changed files with 142 additions and 148 deletions.
1 change: 0 additions & 1 deletion .clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ Checks: |
-clang-analyzer-core.NullDereference,
-clang-analyzer-core.uninitialized.UndefReturn,
-clang-analyzer-optin.mpi.MPI-Checker,
-clang-analyzer-security.FloatLoopCounter,
bugprone-*,
-bugprone-easily-swappable-parameters,
-bugprone-implicit-widening-of-multiplication-result,
Expand Down
8 changes: 4 additions & 4 deletions src/core/actor/visitors.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ struct GetActorName : public boost::static_visitor<std::string> {

/** @brief Get the symbol name of an actor. */
template <class Variant> auto get_actor_name(Variant const &variant) {
return boost::apply_visitor(GetActorName{}, variant);
return boost::apply_visitor(GetActorName(), variant);
}

/** @brief Get an actor of a specific type, recursively. */
Expand Down Expand Up @@ -76,7 +76,7 @@ struct GetActorByType : public boost::static_visitor<std::shared_ptr<Actor>> {
/** @brief Get an active actor of a specific type, recursively. */
template <typename Actor, typename Variant>
std::shared_ptr<Actor> get_actor_by_type(Variant const &variant) {
return boost::apply_visitor(GetActorByType<Actor>{}, variant);
return boost::apply_visitor(GetActorByType<Actor>(), variant);
}

template <typename Actor, typename Variant>
Expand Down Expand Up @@ -118,7 +118,7 @@ struct HasActorOfType : public boost::static_visitor<bool> {
/** @brief Check if an actor of a specific type is active, recursively. */
template <typename Actor, typename Variant>
auto has_actor_of_type(Variant const &variant) {
return boost::apply_visitor(HasActorOfType<Actor>{}, variant);
return boost::apply_visitor(HasActorOfType<Actor>(), variant);
}

template <typename Actor, typename Variant>
Expand Down Expand Up @@ -149,7 +149,7 @@ struct ActorEquality : public boost::static_visitor<bool> {
template <typename T, class Variant>
bool is_already_stored(std::shared_ptr<T> const &actor,
boost::optional<Variant> const &active_actor) {
auto const visitor = std::bind(ActorEquality{}, actor, std::placeholders::_1);
auto const visitor = std::bind(ActorEquality(), actor, std::placeholders::_1);
return active_actor and boost::apply_visitor(visitor, *active_actor);
}

Expand Down
2 changes: 1 addition & 1 deletion src/core/bond_breakage/bond_breakage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ void process_queue() {

// Execute actions
for (auto const &a : actions) {
boost::apply_visitor(execute{}, a);
boost::apply_visitor(execute(), a);
}
}
} // namespace BondBreakage
10 changes: 5 additions & 5 deletions src/core/electrostatics/coulomb.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ struct LongRangePressure : public boost::static_visitor<Utils::Vector9d> {

Utils::Vector9d calc_pressure_long_range(ParticleRange const &particles) {
if (electrostatics_actor) {
return boost::apply_visitor(LongRangePressure{particles},
return boost::apply_visitor(LongRangePressure(particles),
*electrostatics_actor);
}
return {};
Expand Down Expand Up @@ -170,7 +170,7 @@ struct ShortRangeCutoff : public boost::static_visitor<double> {

double cutoff() {
if (electrostatics_actor) {
return boost::apply_visitor(ShortRangeCutoff{}, *electrostatics_actor);
return boost::apply_visitor(ShortRangeCutoff(), *electrostatics_actor);
}
return -1.0;
}
Expand All @@ -191,7 +191,7 @@ struct EventOnObservableCalc : public boost::static_visitor<void> {

void on_observable_calc() {
if (electrostatics_actor) {
boost::apply_visitor(EventOnObservableCalc{}, *electrostatics_actor);
boost::apply_visitor(EventOnObservableCalc(), *electrostatics_actor);
}
}

Expand Down Expand Up @@ -282,7 +282,7 @@ struct LongRangeEnergy : public boost::static_visitor<double> {

void calc_long_range_force(ParticleRange const &particles) {
if (electrostatics_actor) {
boost::apply_visitor(LongRangeForce{particles}, *electrostatics_actor);
boost::apply_visitor(LongRangeForce(particles), *electrostatics_actor);
}
#ifdef ELECTROKINETICS
/* Add fields from EK if enabled */
Expand All @@ -294,7 +294,7 @@ void calc_long_range_force(ParticleRange const &particles) {

double calc_energy_long_range(ParticleRange const &particles) {
if (electrostatics_actor) {
return boost::apply_visitor(LongRangeEnergy{particles},
return boost::apply_visitor(LongRangeEnergy(particles),
*electrostatics_actor);
}
return 0.;
Expand Down
8 changes: 4 additions & 4 deletions src/core/electrostatics/coulomb_inline.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ struct ShortRangeForceCorrectionsKernel
inline ShortRangeForceKernel::result_type pair_force_kernel() {
#ifdef ELECTROSTATICS
if (electrostatics_actor) {
auto const visitor = ShortRangeForceKernel{};
auto const visitor = ShortRangeForceKernel();
return boost::apply_visitor(visitor, *electrostatics_actor);
}
#endif // ELECTROSTATICS
Expand Down Expand Up @@ -180,7 +180,7 @@ struct ShortRangeEnergyKernel
inline ShortRangeForceCorrectionsKernel::result_type pair_force_elc_kernel() {
#ifdef ELECTROSTATICS
if (electrostatics_actor) {
auto const visitor = ShortRangeForceCorrectionsKernel{};
auto const visitor = ShortRangeForceCorrectionsKernel();
return boost::apply_visitor(visitor, *electrostatics_actor);
}
#endif // ELECTROSTATICS
Expand All @@ -190,7 +190,7 @@ inline ShortRangeForceCorrectionsKernel::result_type pair_force_elc_kernel() {
inline ShortRangePressureKernel::result_type pair_pressure_kernel() {
#ifdef ELECTROSTATICS
if (electrostatics_actor) {
auto const visitor = ShortRangePressureKernel{};
auto const visitor = ShortRangePressureKernel();
return boost::apply_visitor(visitor, *electrostatics_actor);
}
#endif // ELECTROSTATICS
Expand All @@ -200,7 +200,7 @@ inline ShortRangePressureKernel::result_type pair_pressure_kernel() {
inline ShortRangeEnergyKernel::result_type pair_energy_kernel() {
#ifdef ELECTROSTATICS
if (electrostatics_actor) {
auto const visitor = ShortRangeEnergyKernel{};
auto const visitor = ShortRangeEnergyKernel();
return boost::apply_visitor(visitor, *electrostatics_actor);
}
#endif // ELECTROSTATICS
Expand Down
4 changes: 2 additions & 2 deletions src/core/electrostatics/icc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ void ICCStar::iteration(CellStructure &cell_structure,
}

auto const prefactor =
boost::apply_visitor(GetCoulombPrefactor{}, *electrostatics_actor);
boost::apply_visitor(GetCoulombPrefactor(), *electrostatics_actor);
auto const pref = 1. / (prefactor * 2. * Utils::pi());
auto const kernel = Coulomb::pair_force_kernel();
auto const elc_kernel = Coulomb::pair_force_elc_kernel();
Expand Down Expand Up @@ -268,7 +268,7 @@ void ICCStar::sanity_check() const {

void ICCStar::sanity_checks_active_solver() const {
if (electrostatics_actor) {
boost::apply_visitor(SanityChecksICC{}, *electrostatics_actor);
boost::apply_visitor(SanityChecksICC(), *electrostatics_actor);
} else {
throw std::runtime_error("An electrostatics solver is needed by ICC");
}
Expand Down
46 changes: 22 additions & 24 deletions src/core/electrostatics/mmm1d.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -326,32 +326,30 @@ void CoulombMMM1D::tune() {
auto const maxrad = box_geo.length()[2];
auto min_time = std::numeric_limits<double>::infinity();
auto min_rad = -1.;
double switch_radius;
auto switch_radius = 0.2 * maxrad;
/* determine optimal switching radius. Should be around 0.33 */
for (switch_radius = 0.2 * maxrad; switch_radius < 0.4 * maxrad;
switch_radius += 0.025 * maxrad) {
if (switch_radius <= bessel_radii.back()) {
// this switching radius is too small for our Bessel series
continue;
}

far_switch_radius_sq = Utils::sqr(switch_radius);
on_coulomb_change();

/* perform force calculation test */
auto const int_time = benchmark_integration_step(tune_timings);

if (tune_verbose) {
std::printf("r= %f t= %f ms\n", switch_radius, int_time);
}

if (int_time < min_time) {
min_time = int_time;
min_rad = switch_radius;
} else if (int_time > 2. * min_time) {
// simple heuristic to skip remaining radii when performance is dropping
break;
while (switch_radius < 0.4 * maxrad) {
if (switch_radius > bessel_radii.back()) {
// this switching radius is large enough for our Bessel series
far_switch_radius_sq = Utils::sqr(switch_radius);
on_coulomb_change();

/* perform force calculation test */
auto const int_time = benchmark_integration_step(tune_timings);

if (tune_verbose) {
std::printf("r= %f t= %f ms\n", switch_radius, int_time);
}

if (int_time < min_time) {
min_time = int_time;
min_rad = switch_radius;
} else if (int_time > 2. * min_time) {
// simple heuristic to skip remaining radii when performance drops
break;
}
}
switch_radius += 0.025 * maxrad;
}
switch_radius = min_rad;
far_switch_radius_sq = Utils::sqr(switch_radius);
Expand Down
7 changes: 3 additions & 4 deletions src/core/electrostatics/mmm1d_gpu_cuda.cu
Original file line number Diff line number Diff line change
Expand Up @@ -250,17 +250,16 @@ void CoulombMMM1DGpu::tune(double maxPWerror, double far_switch_radius,
auto const maxrad = host_boxz;
auto bestrad = 0.0;
float besttime = INFINITY;

// NOLINTNEXTLINE(clang-analyzer-security.FloatLoopCounter)
for (auto radius = 0.05 * maxrad; radius < maxrad;
radius += 0.05 * maxrad) {
auto radius = 0.05 * maxrad;
while (radius < maxrad) {
set_params(0, 0, maxPWerror, radius, bessel_cutoff);
tune(maxPWerror, radius, -2); // tune Bessel cutoff
auto const runtime = force_benchmark();
if (runtime < besttime) {
besttime = runtime;
bestrad = radius;
}
radius += 0.05 * maxrad;
}
set_params(0, 0, maxPWerror, bestrad, bessel_cutoff);
tune(maxPWerror, bestrad, -2); // tune Bessel cutoff
Expand Down
39 changes: 19 additions & 20 deletions src/core/electrostatics/p3m.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -657,8 +657,8 @@ class CoulombTuningAlgorithm : public TuningAlgorithm {
TuningAlgorithm::Parameters get_time() override {
auto tuned_params = TuningAlgorithm::Parameters{};
auto time_best = time_sentinel;
for (auto mesh_density = m_mesh_density_min;
mesh_density <= m_mesh_density_max; mesh_density += 0.1) {
auto mesh_density = m_mesh_density_min;
while (mesh_density <= m_mesh_density_max) {
auto trial_params = TuningAlgorithm::Parameters{};
if (m_tune_mesh) {
for (int i : {0, 1, 2}) {
Expand All @@ -676,26 +676,25 @@ class CoulombTuningAlgorithm : public TuningAlgorithm {
get_m_time(trial_params.mesh, trial_params.cao, trial_params.r_cut_iL,
trial_params.alpha_L, trial_params.accuracy);

/* this mesh does not work at all */
if (trial_time < 0.)
continue;

/* the optimum r_cut for this mesh is the upper limit for higher meshes,
everything else is slower */
if (has_actor_of_type<CoulombP3M>(electrostatics_actor)) {
m_r_cut_iL_max = trial_params.r_cut_iL;
}
if (trial_time >= 0.) {
/* the optimum r_cut for this mesh is the upper limit for higher meshes,
everything else is slower */
if (has_actor_of_type<CoulombP3M>(electrostatics_actor)) {
m_r_cut_iL_max = trial_params.r_cut_iL;
}

if (trial_time < time_best) {
/* new optimum */
reset_n_trials();
tuned_params = trial_params;
time_best = tuned_params.time = trial_time;
} else if (trial_time > time_best + time_granularity or
get_n_trials() > max_n_consecutive_trials) {
/* no hope of further optimisation */
break;
if (trial_time < time_best) {
/* new optimum */
reset_n_trials();
tuned_params = trial_params;
time_best = tuned_params.time = trial_time;
} else if (trial_time > time_best + time_granularity or
get_n_trials() > max_n_consecutive_trials) {
/* no hope of further optimisation */
break;
}
}
mesh_density += 0.1;
}
return tuned_params;
}
Expand Down
23 changes: 11 additions & 12 deletions src/core/forces_inline.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -304,9 +304,10 @@ inline bool add_bonded_two_body_force(
if (auto const *iap = boost::get<ThermalizedBond>(&iaparams)) {
auto result = iap->forces(p1, p2, dx);
if (result) {
using std::get;
p1.force() += get<0>(result.get());
p2.force() += get<1>(result.get());
auto const &forces = result.get();

p1.force() += std::get<0>(forces);
p2.force() += std::get<1>(forces);

return false;
}
Expand Down Expand Up @@ -358,12 +359,11 @@ inline bool add_bonded_three_body_force(Bonded_IA_Parameters const &iaparams,
}
auto const result = calc_bonded_three_body_force(iaparams, p1, p2, p3);
if (result) {
using std::get;
auto const &forces = result.get();

p1.force() += get<0>(forces);
p2.force() += get<1>(forces);
p3.force() += get<2>(forces);
p1.force() += std::get<0>(forces);
p2.force() += std::get<1>(forces);
p3.force() += std::get<2>(forces);

return false;
}
Expand Down Expand Up @@ -397,13 +397,12 @@ inline bool add_bonded_four_body_force(Bonded_IA_Parameters const &iaparams,
Particle &p4) {
auto const result = calc_bonded_four_body_force(iaparams, p1, p2, p3, p4);
if (result) {
using std::get;
auto const &forces = result.get();

p1.force() += get<0>(forces);
p2.force() += get<1>(forces);
p3.force() += get<2>(forces);
p4.force() += get<3>(forces);
p1.force() += std::get<0>(forces);
p2.force() += std::get<1>(forces);
p3.force() += std::get<2>(forces);
p4.force() += std::get<3>(forces);

return false;
}
Expand Down
2 changes: 0 additions & 2 deletions src/core/io/writer/h5md_core.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,6 @@ void File::write(const ParticleRange &particles, double time, int step,

void File::write_connectivity(const ParticleRange &particles) {
MultiArray3i bond(boost::extents[0][0][0]);
int particle_index = 0;
for (auto const &p : particles) {
auto nbonds_local = static_cast<decltype(bond)::index>(bond.shape()[1]);
for (auto const b : p.bonds()) {
Expand All @@ -452,7 +451,6 @@ void File::write_connectivity(const ParticleRange &particles) {
nbonds_local++;
}
}
particle_index++;
}

auto const n_bonds_local = static_cast<int>(bond.shape()[1]);
Expand Down
6 changes: 4 additions & 2 deletions src/core/magnetostatics/dipolar_direct_sum.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,8 @@ void DipolarDirectSum::add_long_range_forces(

/* Range of particles we calculate the ia for on this node */
auto const local_posmom_begin = all_posmom.begin() + offset;
auto const local_posmom_end = local_posmom_begin + local_particles.size();
auto const local_posmom_end =
local_posmom_begin + static_cast<long>(local_particles.size());

/* Output iterator for the force */
auto p = local_particles.begin();
Expand Down Expand Up @@ -379,7 +380,8 @@ DipolarDirectSum::long_range_energy(ParticleRange const &particles) const {

/* Range of particles we calculate the ia for on this node */
auto const local_posmom_begin = all_posmom.begin() + offset;
auto const local_posmom_end = local_posmom_begin + local_particles.size();
auto const local_posmom_end =
local_posmom_begin + static_cast<long>(local_particles.size());

auto u = 0.;
for (auto it = local_posmom_begin; it != local_posmom_end; ++it) {
Expand Down
4 changes: 2 additions & 2 deletions src/core/magnetostatics/dipoles.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -188,13 +188,13 @@ struct LongRangeEnergy : public boost::static_visitor<double> {

void calc_long_range_force(ParticleRange const &particles) {
if (magnetostatics_actor) {
boost::apply_visitor(LongRangeForce{particles}, *magnetostatics_actor);
boost::apply_visitor(LongRangeForce(particles), *magnetostatics_actor);
}
}

double calc_energy_long_range(ParticleRange const &particles) {
if (magnetostatics_actor) {
return boost::apply_visitor(LongRangeEnergy{particles},
return boost::apply_visitor(LongRangeEnergy(particles),
*magnetostatics_actor);
}
return 0.;
Expand Down
Loading

0 comments on commit ff8a010

Please sign in to comment.