0

as an exercise, i'm translating my master's thesis finite-difference time-domain code for simulation of wave propagation from matlab to c++ and i've come across the following problem.

i would like to create a class that corresponds to a non-physical absorbing layer called cpml. the size of the layer depends on the desired parameters of the simulation, so the arrays that define the absorbing layer have to be dynamic.

#ifndef fdtd_h
#define fdtd_h

#include <cmath>
#include <iostream>
#include <sstream>

using namespace std;

class cpml {

public:

int thickness;
int n_1, n_2, n_3;
double cut_off_freq;
double kappa_x_max, sigma_x_1_max, sigma_x_2_max, alpha_x_max;

double *kappa_x_tau_xy, *sigma_x_tau_xy, *alpha_x_tau_xy;

void set_cpml_parameters_tau_xy();

};

void cpml::set_cpml_parameters_tau_xy(){

double temp1[thickness], temp2[thickness], temp3[thickness];

for(int j = 1; j < thickness; j++){

    temp1[j] = 1 + kappa_x_max * pow((double)(thickness - j - 0.5) / (double)(thickness - 1), n_1);
    temp2[j] = sigma_x_1_max * pow((double)(thickness - j - 0.5) / (double)(thickness - 1), n_1 + n_2);
    temp3[j] = alpha_x_max * pow((double)(j - 0.5) / (double)(thickness - 1), n_3);


}

kappa_x_tau_xy = temp1;
sigma_x_tau_xy = temp2;

for(int i = 1; i < thickness; i++){

    cout << sigma_x_tau_xy[i] << endl;

}

alpha_x_tau_xy = temp3;

}

#endif /* fdtd_h */

when i call the function cpml::set_cpml_parameters_tau_xy() in my main function, the first value of the array sigma_x_tau_xy is correct. however, the further values aren't.

#include "fdtd.h"

using namespace std;

int main() {

cpml cpml;

int cpml_thickness = 10;
cpml.thickness = cpml_thickness;

int n_1 = 3, n_2 = 0, n_3 = 3;
cpml.n_1 = n_1; cpml.n_2 = n_2; cpml.n_3 = n_3;

double cut_off_freq = 1;
cpml.cut_off_freq = cut_off_freq;

double kappa_x_max = 0;
double sigma_x_1_max = 0.8 * (n_1 + 1) / (sqrt(simulation_medium.mu/simulation_medium.rho) * simulation_grid.big_delta_x), sigma_x_2_max = 0.8 * (n_1 + 1) / (sqrt(simulation_medium.mu/simulation_medium.rho) * simulation_grid.big_delta_x);
double alpha_x_max = 2 * PI * cpml.cut_off_freq;

double kappa_y_max = 0;
double sigma_y_1_max = 0.8 * (n_1 + 1) / (sqrt(simulation_medium.mu/simulation_medium.rho) * simulation_grid.big_delta_y), sigma_y_2_max = 0.8 * (n_1 + 1) / (sqrt(simulation_medium.mu/simulation_medium.rho) * simulation_grid.big_delta_y);
double alpha_y_max = 2 * PI * cpml.cut_off_freq;

cpml.kappa_x_max = kappa_x_max; cpml.sigma_x_1_max = sigma_x_1_max; cpml.sigma_x_2_max = sigma_x_2_max; cpml.alpha_x_max = alpha_x_max;
cpml.kappa_y_max = kappa_y_max; cpml.sigma_y_1_max = sigma_y_1_max; cpml.sigma_y_2_max = sigma_y_2_max; cpml.alpha_y_max = alpha_y_max;

cpml.set_cpml_parameters_tau_xy();

for(int j = 1; j < cpml.thickness; j++){

    cout << *(cpml.sigma_x_tau_xy + j) << endl;

}

}

what am i doing wrong and how do i make the dynamic array members of the class cpml contain the correct values when called in the main function?

  • 1
    You really should use a `std::vector` if you need a dynamic array. – NathanOliver Apr 05 '17 at 11:46
  • The address of `double temp1[thickness]` is invalid once `set_cpml_parameters_tau_xy` ends, so you can't "store" that for use later. The "best" C++ way would be to use `std::vector` (which is a dynamic container class) instead of raw arrays (which are never "dynamic" but could be dynamically allocated). – crashmstr Apr 05 '17 at 11:47

2 Answers2

3

Two problems: The lesser of them is that your program is technically not a valid C++ program, since C++ doesn't have variable-length arrays (which your arrays temp1, temp2 and temp3 are).

The more serious problem is that you save pointers to local variables. When a function returns, local variables go out of scope and no longer exist. Pointers to them will become invalid, and using those pointers will lead to undefined behavior.

Both problems are easily solved by using std::vector instead of arrays and pointers.

Some programmer dude
  • 400,186
  • 35
  • 402
  • 621
0

You cannot declare an array in C++ without a "constant" expression for its size (the bounds must be known at compile time). That means this code is invalid:

double temp1[thickness], temp2[thickness], temp3[thickness];

What you should instead do is the following:

class cmpl
{
   //...
   std::vector<double> kappa_x_tau_xy, sigma_x_tau_xy, alpha_x_tau_xy;
   // ...
};

void cpml::set_cpml_parameters_tau_xy(){
   alpha_x_tau_xy.resize(thickness);
   kappa_x_tau_xy.resize(thickness);
   sigma_x_tau_xy.resize(thickness);
   //...

std::vector will handle all the dynamic allocation under the hood for you. If your code compiled, it was because you were using a nonstandard GCC extension for variable length arrays. Turn your warnings up -Wall -pedantic -Werror when you compile and it should complain more.

Note that you also have issues in array bounds. Whereas Matlab is 1-indexed, C++ is 0-indexed, so you'll need to do this, too:

for(int j = 0; j < thickness; j++){ 
    alpha_x_tau_xy[j] = 1 + kappa_x_max * pow((double)(thickness - j - 0.5) / (double)(thickness - 1), n_1);
    kappa_x_tau_xy = sigma_x_1_max * pow((double)(thickness - j - 0.5) / (double)(thickness - 1), n_1 + n_2);
    sigma_x_tau_xy = alpha_x_max * pow((double)(j - 0.5) / (double)(thickness - 1), n_3);
}

You have a similar issue in main:

for(int j = 1; j < cpml.thickness; j++){

    cout << *(cpml.sigma_x_tau_xy + j) << endl;

}

Should become:

for(int j = 0; j < cpml.thickness; j++){
    cout << cpml.sigma_x_tau_xy[j] << endl;
}

Additional Notes:

  • Your code is very unstructured. Consider putting all of the cmpl-related getting and setting into the cmpl class ([Encapsulation])(https://en.wikipedia.org/wiki/Encapsulation_(computer_programming)). This will make it easer for the client (you in this case) to interact with the object.

    • This will include hiding your class data as protected or private and exposing functions to get and set those variables (don't forget const where appropriate).
    • Add a constructor to initialize all of the fields at once. As it stands now, your class consists of mostly uninitialized garbage for much of its lifetime. If someone where to prematurely try to access a field, you're in Undefined Behavior territory.
  • std::endl is good for printing newline characters, but restrict that to Debug-only code. The reason being is that it flushes the buffer every time its called, which can make your code overall slower if it's printing a lot. Use a newline character "\n" instead for Release.

  • An additional benefit of std::vector is that it makes copying and assigning to a cmpl well behaved. Otherwise, the compiler will generate a copy constructor and copy assignment, which when used will be a shallow copy instead of the deep copy that you'd want.

After restructuring your class, your main might look something like this:

int main() {
    int cpml_thickness = 10; 
    int n_1 = 3, n_2 = 0, n_3 = 3;  
    double cut_off_freq = 1;

    double kappa_x_max = 0;
    double sigma_x_1_max = 0.8 * (n_1 + 1) / (sqrt(simulation_medium.mu/simulation_medium.rho) * simulation_grid.big_delta_x), sigma_x_2_max = 0.8 * (n_1 + 1) / (sqrt(simulation_medium.mu/simulation_medium.rho) * simulation_grid.big_delta_x);
    double alpha_x_max = 2 * PI * cut_off_freq;

    double kappa_y_max = 0;
    double sigma_y_1_max = 0.8 * (n_1 + 1) / (sqrt(simulation_medium.mu/simulation_medium.rho) * simulation_grid.big_delta_y), sigma_y_2_max = 0.8 * (n_1 + 1) / (sqrt(simulation_medium.mu/simulation_medium.rho) * simulation_grid.big_delta_y);
    double alpha_y_max = 2 * PI * cut_off_freq;

    cpml cpml(cpml_thickness, n_1, n_2, n_3, cut_off_freq, kappa_x_max, kappa_y_max, sigma_x_1_max, sigma_x_2max, alpha_x_max, alpha_y_max);
    cpml.set_cpml_parameters_tau_xy();
    cpml.PrintSigmaTauXY(std::cout);
}

Which is arguably better. (You might use a getter to get sigma_tau_xy from the class and then print it yourself, though). And then you can think about how to simplify things even further by creating objects that represent the logical groupings of alpha_x_max and alpha_y_max etc. This could be a std::pair or a full-on struct with its own getters and setters. Now their own logic is grouped together and is easy to pass around/reference/think about. Your constructor for cmpl also becomes simpler, where you accept a single parameter that represents both x and y instead of separate ones for both.

Matlab doesn't really encourage an Object-Oriented approach in my (admittedly breif) experience, but in C++ it's easy.

Community
  • 1
  • 1
AndyG
  • 39,700
  • 8
  • 109
  • 143