Skip to content

Commit

Permalink
Mark single argument constructors explicit (#93)
Browse files Browse the repository at this point in the history
* Mark single argument constructors explicit
* Remove virtual + override combos
  • Loading branch information
zfergus committed Feb 23, 2024
1 parent 7474c7c commit e370783
Show file tree
Hide file tree
Showing 18 changed files with 45 additions and 39 deletions.
2 changes: 1 addition & 1 deletion src/ipc/broad_phase/spatial_hash.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class SpatialHash : public BroadPhase {
double built_in_radius;

public: // constructor
SpatialHash() { }
SpatialHash() = default;

SpatialHash(
const Eigen::MatrixXd& vertices,
Expand Down
2 changes: 1 addition & 1 deletion src/ipc/candidates/candidates.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ namespace ipc {

class Candidates {
public:
Candidates() { }
Candidates() = default;

/// @brief Initialize the set of discrete collision detection candidates.
/// @param mesh The surface of the collision mesh.
Expand Down
2 changes: 1 addition & 1 deletion src/ipc/candidates/continuous_collision_candidate.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ namespace ipc {
/// Virtual class for candidates that support CCD.
class ContinuousCollisionCandidate : virtual public CollisionStencil {
public:
virtual ~ContinuousCollisionCandidate() { }
virtual ~ContinuousCollisionCandidate() = default;

/// @brief Perform narrow-phase CCD on the candidate.
/// @param vertices_t0 Stencil vertices at the start of the time step.
Expand Down
2 changes: 1 addition & 1 deletion src/ipc/ccd/additive_ccd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ bool additive_ccd(
const double tmax,
const double conservative_rescaling)
{
assert(conservative_rescaling > 0 && conservative_rescaling < 1);
assert(conservative_rescaling > 0 && conservative_rescaling <= 1);

const double min_distance_sq = min_distance * min_distance;

Expand Down
4 changes: 2 additions & 2 deletions src/ipc/collision_mesh.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class CollisionMesh {
public:
/// @brief Construct a new Collision Mesh object.
/// Collision Mesh objects are immutable, so use the other constructors.
CollisionMesh() { }
CollisionMesh() = default;

/// @brief Construct a new Collision Mesh object directly from the collision mesh vertices.
/// @param rest_positions The vertices of the collision mesh at rest (#V × dim).
Expand Down Expand Up @@ -64,7 +64,7 @@ class CollisionMesh {
void init_area_jacobians();

/// @brief Destroy the Collision Mesh object
~CollisionMesh() { }
~CollisionMesh() = default;

/// @brief Get the number of vertices in the collision mesh.
size_t num_vertices() const { return m_vertex_to_full_vertex.size(); }
Expand Down
2 changes: 1 addition & 1 deletion src/ipc/collisions/collision.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class Collision : virtual public CollisionStencil {
const double weight,
const Eigen::SparseVector<double>& weight_gradient);

virtual ~Collision() { }
virtual ~Collision() = default;

// -- Distance mollifier ---------------------------------------------------

Expand Down
2 changes: 1 addition & 1 deletion src/ipc/collisions/collisions.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class Collisions {
using value_type = Collision;

public:
Collisions() { }
Collisions() = default;

/// @brief Initialize the set of collisions used to compute the barrier potential.
/// @param mesh The collision mesh.
Expand Down
6 changes: 3 additions & 3 deletions src/ipc/collisions/edge_edge.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class EdgeEdgeCollision : public EdgeEdgeCandidate, public Collision {
/// @param positions The stencil's vertex positions.
/// @param eps_x The mollifier's tolerance.
/// @return The mollifier gradient.
virtual VectorMax12d mollifier_gradient(
VectorMax12d mollifier_gradient(
const VectorMax12d& positions, double eps_x) const override;

/// @brief Compute the Hessian of the mollifier for the distance w.r.t. positions.
Expand All @@ -71,7 +71,7 @@ class EdgeEdgeCollision : public EdgeEdgeCandidate, public Collision {
/// @param positions The stencil's vertex positions.
/// @param eps_x The mollifier's tolerance.
/// @return The mollifier Hessian.
virtual MatrixMax12d mollifier_hessian(
MatrixMax12d mollifier_hessian(
const VectorMax12d& positions, double eps_x) const override;

/// @brief Compute the gradient of the mollifier for the distance w.r.t. rest positions.
Expand All @@ -92,7 +92,7 @@ class EdgeEdgeCollision : public EdgeEdgeCandidate, public Collision {

// ------------------------------------------------------------------------

virtual EdgeEdgeDistanceType known_dtype() const override { return dtype; }
EdgeEdgeDistanceType known_dtype() const override { return dtype; }

// ------------------------------------------------------------------------

Expand Down
2 changes: 1 addition & 1 deletion src/ipc/friction/collisions/friction_collision.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class FrictionCollision : virtual public CollisionStencil {
const double barrier_stiffness);

public:
virtual ~FrictionCollision() { }
virtual ~FrictionCollision() = default;

/// @brief Get the dimension of the collision.
int dim() const { return tangent_basis.rows(); }
Expand Down
2 changes: 1 addition & 1 deletion src/ipc/friction/friction_collisions.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class FrictionCollisions {
using value_type = FrictionCollision;

public:
FrictionCollisions() { }
FrictionCollisions() = default;

void build(
const CollisionMesh& mesh,
Expand Down
2 changes: 1 addition & 1 deletion src/ipc/potentials/barrier_potential.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class BarrierPotential : public DistanceBasedPotential {
public:
/// @brief Construct a barrier potential.
/// @param dhat The activation distance of the barrier.
BarrierPotential(const double dhat);
explicit BarrierPotential(const double dhat);

/// @brief Construct a barrier potential.
/// @param dhat The activation distance of the barrier.
Expand Down
4 changes: 2 additions & 2 deletions src/ipc/potentials/distance_based_potential.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ class DistanceBasedPotential : public Potential<Collisions> {
using Super = Potential<Collisions>;

public:
DistanceBasedPotential() { }
virtual ~DistanceBasedPotential() { }
DistanceBasedPotential() = default;
virtual ~DistanceBasedPotential() = default;

// -- Cumulative methods ---------------------------------------------------

Expand Down
2 changes: 1 addition & 1 deletion src/ipc/potentials/friction_potential.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class FrictionPotential : public Potential<FrictionCollisions> {
public:
/// @brief Construct a friction potential.
/// @param epsv The smooth friction mollifier parameter \f$\epsilon_v\f$.
FrictionPotential(const double epsv);
explicit FrictionPotential(const double epsv);

/// @brief Get the smooth friction mollifier parameter \f$\epsilon_v\f$.
double epsv() const { return m_epsv; }
Expand Down
4 changes: 2 additions & 2 deletions src/ipc/potentials/potential.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ template <class TCollisions> class Potential {
using TCollision = typename TCollisions::value_type;

public:
Potential() { }
virtual ~Potential() { }
Potential() = default;
virtual ~Potential() = default;

// -- Cumulative methods ---------------------------------------------------

Expand Down
2 changes: 1 addition & 1 deletion tests/src/tests/ccd/test_edge_edge_ccd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ TEST_CASE("Edge-Edge CCD", "[ccd][3D][edge-edge]")
is_collision_expected = dy >= d0 / 2;
conservative_check = false;
}
CAPTURE(int(is_collision_expected));
CAPTURE(is_collision_expected);

double toi;
bool is_colliding = edge_edge_ccd(
Expand Down
39 changes: 22 additions & 17 deletions tests/src/tests/friction/test_force_jacobian.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ void check_friction_force_jacobian(

FrictionCollisions friction_collisions;
friction_collisions.build(
mesh, X + Ut, collisions, dhat, barrier_stiffness, mu);
mesh, X + Ut, collisions, BarrierPotential(dhat), barrier_stiffness,
mu);
CHECK(friction_collisions.size());

const FrictionPotential D(epsv_times_h);
Expand Down Expand Up @@ -89,8 +90,8 @@ void check_friction_force_jacobian(
///////////////////////////////////////////////////////////////////////////

Eigen::MatrixXd JF_wrt_X = D.force_jacobian(
friction_collisions, mesh, X, Ut, velocities, dhat, barrier_stiffness,
FrictionPotential::DiffWRT::REST_POSITIONS);
friction_collisions, mesh, X, Ut, velocities, BarrierPotential(dhat),
barrier_stiffness, FrictionPotential::DiffWRT::REST_POSITIONS);

auto F_X = [&](const Eigen::VectorXd& x) {
Eigen::MatrixXd fd_X = fd::unflatten(x, X.cols());
Expand All @@ -107,14 +108,15 @@ void check_friction_force_jacobian(
fd_collisions.build(fd_mesh, fd_X + Ut, dhat);

fd_friction_collisions.build(
fd_mesh, fd_X + Ut, fd_collisions, dhat, barrier_stiffness, mu);
fd_mesh, fd_X + Ut, fd_collisions, BarrierPotential(dhat),
barrier_stiffness, mu);
} else {
fd_friction_collisions = friction_collisions;
}

return D.force(
fd_friction_collisions, fd_mesh, fd_X, Ut, velocities, dhat,
barrier_stiffness);
fd_friction_collisions, fd_mesh, fd_X, Ut, velocities,
BarrierPotential(dhat), barrier_stiffness);
};
Eigen::MatrixXd fd_JF_wrt_X;
fd::finite_jacobian(fd::flatten(X), F_X, fd_JF_wrt_X);
Expand All @@ -127,8 +129,8 @@ void check_friction_force_jacobian(
///////////////////////////////////////////////////////////////////////////

Eigen::MatrixXd JF_wrt_Ut = D.force_jacobian(
friction_collisions, mesh, X, Ut, velocities, dhat, barrier_stiffness,
FrictionPotential::DiffWRT::LAGGED_DISPLACEMENTS);
friction_collisions, mesh, X, Ut, velocities, BarrierPotential(dhat),
barrier_stiffness, FrictionPotential::DiffWRT::LAGGED_DISPLACEMENTS);

auto F_Ut = [&](const Eigen::VectorXd& ut) {
Eigen::MatrixXd fd_Ut = fd::unflatten(ut, Ut.cols());
Expand All @@ -142,14 +144,15 @@ void check_friction_force_jacobian(
fd_collisions.build(mesh, X + fd_Ut, dhat);

fd_friction_collisions.build(
mesh, X + fd_Ut, fd_collisions, dhat, barrier_stiffness, mu);
mesh, X + fd_Ut, fd_collisions, BarrierPotential(dhat),
barrier_stiffness, mu);
} else {
fd_friction_collisions = friction_collisions;
}

return D.force(
friction_collisions, mesh, X, fd_Ut, velocities, dhat,
barrier_stiffness);
friction_collisions, mesh, X, fd_Ut, velocities,
BarrierPotential(dhat), barrier_stiffness);
};
Eigen::MatrixXd fd_JF_wrt_Ut;
fd::finite_jacobian(fd::flatten(Ut), F_Ut, fd_JF_wrt_Ut);
Expand All @@ -162,13 +165,14 @@ void check_friction_force_jacobian(
///////////////////////////////////////////////////////////////////////////

Eigen::MatrixXd JF_wrt_V = D.force_jacobian(
friction_collisions, mesh, X, Ut, velocities, dhat, barrier_stiffness,
FrictionPotential::DiffWRT::VELOCITIES);
friction_collisions, mesh, X, Ut, velocities, BarrierPotential(dhat),
barrier_stiffness, FrictionPotential::DiffWRT::VELOCITIES);

auto F_V = [&](const Eigen::VectorXd& v) {
return D.force(
friction_collisions, mesh, X, Ut,
fd::unflatten(v, velocities.cols()), dhat, barrier_stiffness);
fd::unflatten(v, velocities.cols()), BarrierPotential(dhat),
barrier_stiffness);
};
Eigen::MatrixXd fd_JF_wrt_V;
fd::finite_jacobian(fd::flatten(velocities), F_V, fd_JF_wrt_V);
Expand Down Expand Up @@ -198,16 +202,17 @@ void check_friction_force_jacobian(
///////////////////////////////////////////////////////////////////////////

const Eigen::VectorXd force = D.force(
friction_collisions, mesh, X, Ut, velocities, dhat, barrier_stiffness);
friction_collisions, mesh, X, Ut, velocities, BarrierPotential(dhat),
barrier_stiffness);
const Eigen::VectorXd grad_D =
D.gradient(friction_collisions, mesh, velocities);
CHECK(fd::compare_gradient(-force, grad_D));

///////////////////////////////////////////////////////////////////////////

Eigen::MatrixXd jac_force = D.force_jacobian(
friction_collisions, mesh, X, Ut, velocities, dhat, barrier_stiffness,
FrictionPotential::DiffWRT::VELOCITIES);
friction_collisions, mesh, X, Ut, velocities, BarrierPotential(dhat),
barrier_stiffness, FrictionPotential::DiffWRT::VELOCITIES);
CHECK(fd::compare_jacobian(-jac_force, hess_D));
}

Expand Down
3 changes: 2 additions & 1 deletion tests/src/tests/friction/test_friction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,8 @@ TEST_CASE(

FrictionCollisions friction_collisions;
friction_collisions.build(
mesh, V_lagged, collisions, dhat, barrier_stiffness, mu);
mesh, V_lagged, collisions, BarrierPotential(dhat), barrier_stiffness,
mu);
REQUIRE(friction_collisions.size() == collisions.size());
REQUIRE(friction_collisions.size() == expected_friction_collisions.size());

Expand Down
2 changes: 1 addition & 1 deletion tests/src/tests/potential/test_friction_potential.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ TEST_CASE("Friction gradient and hessian", "[friction][gradient][hessian]")

FrictionCollisions friction_collisions;
friction_collisions.build(
mesh, V0, collisions, dhat, barrier_stiffness, mu);
mesh, V0, collisions, BarrierPotential(dhat), barrier_stiffness, mu);

const FrictionPotential D(epsv_times_h);

Expand Down

0 comments on commit e370783

Please sign in to comment.