Skip to content

Commit 124803f

Browse files
dav-ogdav-og
committed
refactor: wave class code quality cleanup
Systematic code quality pass over the wave class hierarchy. Bugs/correctness: const WaveMode mode_ changed to static constexpr (fixes deleted assignment operators); CalculateWidthIRF() moved out of per-body loop; dead spectrum_ member removed; unsigned underflow fixed in get_lower_index; duplicate IRF time-max computation unified; uninitialized RegularWaveParams defaults added; silent cerr replaced with exception. Design: all virtual query methods made const (removes const_cast); WaveBase public data encapsulated behind const getters; AddH5Data SimulationParameters made const&; large-object getters return const&; copy/move deleted on WaveBase to prevent slicing; trailing underscores removed from param struct fields (Google style). Style: kDofsPerBody named constant replaces magic 6; spectrumCreated_ renamed to snake_case; Eigen/Dense narrowed to Eigen/Core; C-style casts replaced with static_cast; time_data_ demoted from member to local. All 25 regression tests pass.
1 parent 130e58c commit 124803f

13 files changed

Lines changed: 211 additions & 210 deletions

‎demos/sphere/demo_sphere_irreg_waves.cpp‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -103,12 +103,12 @@ int main(int argc, char* argv[]) {
103103
bodies.push_back(sphereBody);
104104

105105
IrregularWaveParams wave_inputs;
106-
wave_inputs.ramp_duration_ = 60.0;
107-
wave_inputs.wave_height_ = 2.0;
108-
wave_inputs.wave_period_ = 12.0;
109-
wave_inputs.frequency_min_ = 0.001;
110-
wave_inputs.frequency_max_ = 1.0;
111-
wave_inputs.nfrequencies_ = 1000;
106+
wave_inputs.ramp_duration = 60.0;
107+
wave_inputs.wave_height = 2.0;
108+
wave_inputs.wave_period = 12.0;
109+
wave_inputs.frequency_min = 0.001;
110+
wave_inputs.frequency_max = 1.0;
111+
wave_inputs.nfrequencies = 1000;
112112

113113
std::shared_ptr<IrregularWaves> my_hydro_inputs; // declare outside the try-catch block
114114

‎demos/sphere/demo_sphere_irreg_waves_eta_import.cpp‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -106,11 +106,11 @@ int main(int argc, char* argv[]) {
106106
std::cout << "Defining irregular wave input parameters..." << std::endl;
107107
IrregularWaveParams params;
108108
std::cout << "bodies.size() = " << bodies.size() << std::endl;
109-
params.ramp_duration_ = 0.0;
110-
params.eta_file_path_ = (DATADIR / "demos" / "sphere" / "eta" / "eta.txt").lexically_normal().generic_string();
111-
params.frequency_min_ = 0.001;
112-
params.frequency_max_ = 1.0;
113-
params.nfrequencies_ = 1000;
109+
params.ramp_duration = 0.0;
110+
params.eta_file_path = (DATADIR / "demos" / "sphere" / "eta" / "eta.txt").lexically_normal().generic_string();
111+
params.frequency_min = 0.001;
112+
params.frequency_max = 1.0;
113+
params.nfrequencies = 1000;
114114

115115
std::shared_ptr<IrregularWaves> my_hydro_inputs; // declare outside the try-catch block
116116

‎include/hydroc/waves/irregular_wave.h‎

Lines changed: 24 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -13,17 +13,17 @@
1313
#include <vector>
1414

1515
struct IrregularWaveParams {
16-
double wave_height_ = 0.0;
17-
double wave_period_ = 0.0;
18-
double frequency_min_ = 0.001;
19-
double frequency_max_ = 1.0;
20-
int nfrequencies_ = 0;
21-
double peak_enhancement_factor_ = 1.0;
22-
bool is_normalized_ = false;
23-
int seed_ = 1;
24-
bool wave_stretching_ = true;
25-
double ramp_duration_ = 0.0;
26-
std::string eta_file_path_;
16+
double wave_height = 0.0;
17+
double wave_period = 0.0;
18+
double frequency_min = 0.001;
19+
double frequency_max = 1.0;
20+
int nfrequencies = 0;
21+
double peak_enhancement_factor = 1.0;
22+
bool is_normalized = false;
23+
int seed = 1;
24+
bool wave_stretching = true;
25+
double ramp_duration = 0.0;
26+
std::string eta_file_path;
2727
};
2828

2929
class IrregularWaves : public WaveBase {
@@ -33,25 +33,25 @@ class IrregularWaves : public WaveBase {
3333

3434
void CreateSpectrum();
3535
std::vector<double> GetSpectrum();
36-
std::vector<double> GetFreeSurfaceElevation();
37-
std::vector<double> GetFreeSurfaceTime() const;
36+
const std::vector<double>& GetFreeSurfaceElevation() const;
37+
const std::vector<double>& GetFreeSurfaceTime() const;
3838
std::vector<double> GetFrequenciesHz() const;
3939

4040
std::pair<std::vector<double>, std::vector<double>>
4141
ComputeElevationTimeSeries(double t_start, double t_end, double dt) const;
4242

43-
Eigen::VectorXd GetForceAtTime(double t) override;
44-
WaveMode GetWaveMode() override { return mode_; }
43+
Eigen::VectorXd GetForceAtTime(double t) const override;
44+
WaveMode GetWaveMode() const override { return mode_; }
4545

4646
Eigen::VectorXd SetSpectrumFrequencies(double start, double end, int num_steps);
4747

48-
void AddH5Data(std::vector<HydroData::IrregularWaveInfo>& irreg_h5_data, HydroData::SimulationParameters& sim_data);
48+
void AddH5Data(std::vector<HydroData::IrregularWaveInfo>& irreg_h5_data, const HydroData::SimulationParameters& sim_data);
4949

5050
double GetIRFTimeMax() const { return irf_time_max_; }
5151

52-
double GetElevation(const Eigen::Vector3d& position, double time) override;
53-
Eigen::Vector3d GetVelocity(const Eigen::Vector3d& position, double time, double elevation) override;
54-
Eigen::Vector3d GetAcceleration(const Eigen::Vector3d& position, double time, double elevation) override;
52+
double GetElevation(const Eigen::Vector3d& position, double time) const override;
53+
Eigen::Vector3d GetVelocity(const Eigen::Vector3d& position, double time, double elevation) const override;
54+
Eigen::Vector3d GetAcceleration(const Eigen::Vector3d& position, double time, double elevation) const override;
5555

5656
/// Return the surface slope (∂η/∂x, ∂η/∂y) at a given position and time.
5757
/// Used for computing surface normals in visualization.
@@ -64,11 +64,10 @@ class IrregularWaves : public WaveBase {
6464

6565
private:
6666
IrregularWaveParams params_;
67-
std::vector<double> spectrum_;
68-
bool spectrumCreated_ = false;
67+
bool spectrum_created_ = false;
6968
bool use_eta_from_file_ = false;
7069

71-
const WaveMode mode_ = WaveMode::irregular;
70+
static constexpr WaveMode mode_ = WaveMode::irregular;
7271
std::vector<HydroData::IrregularWaveInfo> wave_info_;
7372
std::vector<Eigen::MatrixXd> ex_irf_sampled_;
7473
std::vector<Eigen::VectorXd> ex_irf_time_sampled_;
@@ -80,7 +79,6 @@ class IrregularWaves : public WaveBase {
8079
Eigen::VectorXd wave_phases_;
8180

8281
// Stored eta data — only populated when importing from file.
83-
std::vector<double> time_data_;
8482
std::vector<double> free_surface_elevation_sampled_;
8583
std::vector<double> free_surface_time_sampled_;
8684

@@ -107,11 +105,11 @@ class IrregularWaves : public WaveBase {
107105
void InitializeIRFVectors();
108106
void PrecomputeAmplitudes();
109107
void PrecomputeExcitationTransfer();
110-
void ReadEtaFromFile();
108+
std::vector<double> ReadEtaFromFile();
111109

112-
Eigen::MatrixXd GetExcitationIRF(int b) const;
110+
const Eigen::MatrixXd& GetExcitationIRF(int b) const;
113111
void CalculateWidthIRF();
114-
double ExcitationConvolution(int body, int dof, double time);
112+
double ExcitationConvolution(int body, int dof, double time) const;
115113
};
116114

117115
#endif // HYDROC_WAVES_IRREGULAR_WAVE_H

‎include/hydroc/waves/regular_wave.h‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,10 @@
1515
// Wave utilities are internal implementation details
1616

1717
struct RegularWaveParams {
18-
double regular_wave_amplitude_;
19-
double regular_wave_omega_;
20-
double regular_wave_phase_ = 0.0;
21-
bool wave_stretching_ = true;
18+
double regular_wave_amplitude = 0.0;
19+
double regular_wave_omega = 0.0;
20+
double regular_wave_phase = 0.0;
21+
bool wave_stretching = true;
2222
};
2323

2424
class RegularWave : public WaveBase {
@@ -27,26 +27,26 @@ class RegularWave : public WaveBase {
2727
explicit RegularWave(const RegularWaveParams& params);
2828

2929
void Initialize() override;
30-
Eigen::VectorXd GetForceAtTime(double t) override;
31-
WaveMode GetWaveMode() override { return mode_; }
30+
Eigen::VectorXd GetForceAtTime(double t) const override;
31+
WaveMode GetWaveMode() const override { return mode_; }
3232

33-
//// RADU - eliminate these and use a RegularWaveParams struct (consistent with IrregularWaves)
33+
// TODO: Eliminate public members; use RegularWaveParams struct (consistent with IrregularWaves).
3434
double regular_wave_amplitude_;
3535
double regular_wave_omega_;
3636
double regular_wave_phase_ = 0.0;
3737

38-
void AddH5Data(std::vector<HydroData::RegularWaveInfo>& reg_h5_data, HydroData::SimulationParameters& sim_data);
38+
void AddH5Data(std::vector<HydroData::RegularWaveInfo>& reg_h5_data, const HydroData::SimulationParameters& sim_data);
3939

40-
double GetElevation(const Eigen::Vector3d& position, double time) override;
41-
Eigen::Vector3d GetVelocity(const Eigen::Vector3d& position, double time, double elevation) override;
42-
Eigen::Vector3d GetAcceleration(const Eigen::Vector3d& position, double time, double elevation) override;
40+
double GetElevation(const Eigen::Vector3d& position, double time) const override;
41+
Eigen::Vector3d GetVelocity(const Eigen::Vector3d& position, double time, double elevation) const override;
42+
Eigen::Vector3d GetAcceleration(const Eigen::Vector3d& position, double time, double elevation) const override;
4343

4444
/// Return the surface slope (∂η/∂x, ∂η/∂y) at a given position and time.
4545
/// Used for computing surface normals in visualization.
4646
Eigen::Vector2d GetElevationGradientXY(const Eigen::Vector3d& position, double time) const;
4747

4848
private:
49-
const WaveMode mode_ = WaveMode::regular;
49+
static constexpr WaveMode mode_ = WaveMode::regular;
5050
std::vector<HydroData::RegularWaveInfo> wave_info_;
5151
Eigen::VectorXd excitation_force_mag_;
5252
Eigen::VectorXd excitation_force_phase_;

‎include/hydroc/waves/wave_base.h‎

Lines changed: 30 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,10 @@
66
#ifndef HYDROC_WAVES_WAVE_BASE_H
77
#define HYDROC_WAVES_WAVE_BASE_H
88

9-
#include <Eigen/Dense>
9+
#include <Eigen/Core>
1010

11-
//// RADU - why is this not in some namespace?
12-
//// - should these classes be DLL-exported (e.g., to allow AddWave(NoWave))?
13-
//// if not, why is this a public headers?
11+
// TODO: Wrap wave classes in a namespace (e.g., hydrochrono::waves).
12+
// TODO: Determine if these classes need DLL-export for external wave model support.
1413

1514
enum class WaveMode {
1615
noWaveCIC = 0,
@@ -36,27 +35,38 @@ enum class WaveMode {
3635
*/
3736
class WaveBase {
3837
public:
38+
static constexpr int kDofsPerBody = 6;
39+
3940
virtual ~WaveBase() = default;
4041

41-
virtual void Initialize() = 0;
42-
virtual Eigen::VectorXd GetForceAtTime(double t) = 0;
43-
virtual WaveMode GetWaveMode() = 0;
44-
virtual double GetElevation(const Eigen::Vector3d& position, double time) = 0;
45-
virtual Eigen::Vector3d GetVelocity(const Eigen::Vector3d& position, double time, double elevation) = 0;
46-
virtual Eigen::Vector3d GetAcceleration(const Eigen::Vector3d& position, double time, double elevation) = 0;
42+
WaveBase(const WaveBase&) = delete;
43+
WaveBase& operator=(const WaveBase&) = delete;
44+
45+
virtual void Initialize() = 0;
46+
virtual Eigen::VectorXd GetForceAtTime(double t) const = 0;
47+
virtual WaveMode GetWaveMode() const = 0;
48+
virtual double GetElevation(const Eigen::Vector3d& position, double time) const = 0;
49+
virtual Eigen::Vector3d GetVelocity(const Eigen::Vector3d& position, double time, double elevation) const = 0;
50+
virtual Eigen::Vector3d GetAcceleration(const Eigen::Vector3d& position, double time, double elevation) const = 0;
4751

48-
Eigen::Vector3d GetVelocity(const Eigen::Vector3d& position, double time);
49-
Eigen::Vector3d GetAcceleration(const Eigen::Vector3d& position, double time);
52+
Eigen::Vector3d GetVelocity(const Eigen::Vector3d& position, double time) const;
53+
Eigen::Vector3d GetAcceleration(const Eigen::Vector3d& position, double time) const;
5054

5155
void SetNumBodies(unsigned int n) { num_bodies_ = n; }
5256
unsigned int GetNumBodies() const { return num_bodies_; }
5357

58+
double GetMWL() const { return mwl_; }
59+
double GetGravity() const { return g_; }
60+
double GetWaterDepth() const { return water_depth_; }
61+
bool GetWaveStretching() const { return wave_stretching_; }
62+
63+
protected:
64+
WaveBase() = default;
65+
5466
double mwl_ = 0.0;
5567
double g_ = 9.81;
5668
double water_depth_ = 0.0;
5769
bool wave_stretching_ = true;
58-
59-
protected:
6070
unsigned int num_bodies_ = 0;
6171
};
6272

@@ -65,14 +75,14 @@ class NoWave : public WaveBase {
6575
NoWave() = default;
6676

6777
void Initialize() override {}
68-
Eigen::VectorXd GetForceAtTime(double t) override;
69-
WaveMode GetWaveMode() override { return mode_; }
70-
double GetElevation(const Eigen::Vector3d&, double) override { return 0.0; }
71-
Eigen::Vector3d GetVelocity(const Eigen::Vector3d&, double, double) override { return Eigen::Vector3d(0.0, 0.0, 0.0); }
72-
Eigen::Vector3d GetAcceleration(const Eigen::Vector3d&, double, double) override { return Eigen::Vector3d(0.0, 0.0, 0.0); }
78+
Eigen::VectorXd GetForceAtTime(double t) const override;
79+
WaveMode GetWaveMode() const override { return mode_; }
80+
double GetElevation(const Eigen::Vector3d&, double) const override { return 0.0; }
81+
Eigen::Vector3d GetVelocity(const Eigen::Vector3d&, double, double) const override { return Eigen::Vector3d(0.0, 0.0, 0.0); }
82+
Eigen::Vector3d GetAcceleration(const Eigen::Vector3d&, double, double) const override { return Eigen::Vector3d(0.0, 0.0, 0.0); }
7383

7484
private:
75-
const WaveMode mode_ = WaveMode::noWaveCIC;
85+
static constexpr WaveMode mode_ = WaveMode::noWaveCIC;
7686
};
7787

7888
#endif // HYDROC_WAVES_WAVE_BASE_H

‎src/gui/guihelperVSG.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -234,10 +234,10 @@ void GUIImplVSG::UpdateRadiationSourceBody(double t) {
234234
// Get wave properties from wave model if available.
235235
if (wave_model_) {
236236
// Water depth and gravity (available in all wave models).
237-
if (wave_model_->water_depth_ > 0.0) {
238-
rad_params.water_depth = wave_model_->water_depth_;
237+
if (wave_model_->GetWaterDepth() > 0.0) {
238+
rad_params.water_depth = wave_model_->GetWaterDepth();
239239
}
240-
rad_params.gravity = wave_model_->g_;
240+
rad_params.gravity = wave_model_->GetGravity();
241241

242242
// Wave period from RegularWave.
243243
if (wave_model_->GetWaveMode() == WaveMode::regular) {

‎src/hydro/config/setup_from_yaml.cpp‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -60,11 +60,11 @@ std::shared_ptr<WaveBase> CreateWaveFromSettings(const WaveSettings& wave_settin
6060

6161
} else if (type == "irregular") {
6262
IrregularWaveParams params;
63-
params.ramp_duration_ = ramp_duration;
64-
params.wave_height_ = wave_settings.height;
65-
params.wave_period_ = wave_settings.period;
66-
params.nfrequencies_ = 1000;
67-
params.seed_ = (wave_settings.seed > 0 ? wave_settings.seed : 1);
63+
params.ramp_duration = ramp_duration;
64+
params.wave_height = wave_settings.height;
65+
params.wave_period = wave_settings.period;
66+
params.nfrequencies = 1000;
67+
params.seed = (wave_settings.seed > 0 ? wave_settings.seed : 1);
6868

6969
auto irregular_wave = std::make_shared<IrregularWaves>(params);
7070

0 commit comments

Comments
 (0)