0

I am trying to use the suggestion from this post to free up time being spent in _platform_memmove$VARIANT$Haswell. According to a time profiler, this is occurring when I send a pointer to several class instances to a function. I have tried changing the way I declare the class instances, changing what the function takes, etc. but have not been able to resolve this.

The chunk of my code that may help:

Inputs *tables = new Inputs(OutputFolder, DataFolder);



ScreenStrat *strat_burnin = new ScreenStrat(ScreenStrat::NoScreen, ScreenStrat::NoScreen,
                                             tables->ScreenStartAge, tables->ScreenStopAgeHIV,
                                             tables->ScreenStopAge, ScreenStrat::NoVaccine);

calibrate *calib_output = new calibrate ();

StateMachine *Machine = new StateMachine();

for (int i = 0; i < n_sims; i++){

    calib_output->saved_output[i] = RunCalibration(calib_output->calib_params[i], *strat_burnin, *tables, *Machine);


}

auto ret_val = *calib_output;

delete strat_burnin;
delete tables;
delete Machine;
delete calib_output;

return(ret_val);

and then the function declaration:

vector<double> RunCalibration(vector<double> calib_params, ScreenStrat &strat_burnin, Inputs &tables, StateMachine &Machine)

EDIT I addressed the points @Botje suggest and it hasn't fixed the problems. Updated code:

void RunCalibration(calibrate &calib, ScreenStrat &strat_burnin, Inputs &tables, StateMachine &Machine, int i);
unique_ptr<calibrate> RunChain(string RunsFileName, string CurKey, string OutputFolder, string DataFolder);
int main(int argc, char* argv[]) {

string DataFolder;
string OutputFolder;


DataFolder = "../Data/";
OutputFolder = "../Output/";


unsigned int run;
string CurKey;
string RunsFileName(DataFolder);

if(argc == 1){
    RunsFileName.append("test.ini");
}
else if(argc > 1){
    RunsFileName.append(argv[1]);
}

CIniFile RunsFile(RunsFileName);
if (!RunsFile.ReadFile()) {
    cout << "Could not read Runs File: " << RunsFileName << endl;
    exit(1);
}

CurKey = RunsFile.GetKeyName (0);

if (RunsFile.GetValue(CurKey, "RunType") == "Calibration"){

    int totaliters = RunsFile.GetValueI(CurKey, "Iterations");
    int n_sims = RunsFile.GetValueI(CurKey, "Simulations");

    vector<future<unique_ptr<calibrate>>> futures;
    vector<unique_ptr<calibrate>> modeloutputs;

    for (run = 0; run < totaliters; run++){
        futures.push_back (async(launch::async, RunChain, RunsFileName, CurKey, OutputFolder, DataFolder));
    }

    for (int i = 0; i < futures.size(); i++){
        modeloutputs.push_back (futures[i].get());
    } return(0)}




unique_ptr<calibrate> RunChain(string RunsFileName, string CurKey, string OutputFolder, string DataFolder) {

Inputs *tables = new Inputs(OutputFolder, DataFolder);
tables->loadRFG (RunsFileName, CurKey);
tables->loadVariables ();

int n_sims = tables->Simulations;
int n_params = tables->Multipliers.size();
int n_targs = tables->CalibTargs.size();

ScreenStrat *strat_burnin = new ScreenStrat(ScreenStrat::NoScreen, ScreenStrat::NoScreen,
                                             tables->ScreenStartAge, tables->ScreenStopAgeHIV,
                                             tables->ScreenStopAge, ScreenStrat::NoVaccine);

calibrate *calib_output = new calibrate (n_sims, n_params, n_targs);


calib_output->multipliers_names = tables->MultipliersNames;
calib_output->calib_targs_names = tables->CalibTargsNames;


for (int i = 0; i < n_targs; i ++){
    calib_output->calib_targs[i] = tables->CalibTargs[i][0];
    calib_output->calib_targs_SD[i] = tables->CalibTargs[i][1];
}

for (int i = 0; i < n_params; i++){
    for (int j = 0; j < 3; j++){
        calib_output->multipliers[i][j] = tables->Multipliers[i][j];
    }
}

StateMachine *Machine = new StateMachine();

for (int i = 0; i < n_sims; i++){


    RunCalibration(*calib_output, *strat_burnin, *tables, *Machine, i);


}

unique_ptr<calibrate> ret_val = make_unique<calibrate>(*calib_output);

delete strat_burnin;
delete tables;
delete Machine;
delete calib_output;
return(ret_val);

}

void RunCalibration(calibrate &calib, ScreenStrat &strat_burnin, Inputs &tables, StateMachine &Machine, int i){

Adding in Calibrate definition per request from @botje

#include "calibrate.h"

using namespace std;


calibrate::calibrate(int n_sims, int n_params, int n_targs) {

calib_targs.resize (n_targs);
calib_targs_SD.resize (n_targs);

multipliers.resize(n_params);
for(int i = 0; i < n_params; i++){
    multipliers[i].resize(3);
}

calib_params.resize (n_sims);
for (int i = 0; i < calib_params.size(); i++){
    calib_params[i].resize (n_params);
}

saved_output.resize (n_sims);
for (int i = 0; i < saved_output.size(); i++){
    saved_output[i].resize (n_targs);
}

best_params.resize (n_params);

GOF.clear();

tuned_SD.resize(n_params);

}

calibrate::~calibrate(void) {

}

void calibrate::CalculateGOF(int n_sims) {

GOF.push_back (WeightedDistance (saved_output[n_sims][0], calib_targs[0], calib_targs_SD[0]));
for (int i = 1; i < calib_targs.size(); i ++){
    GOF[n_sims] += WeightedDistance (saved_output[n_sims][i], calib_targs[i], calib_targs_SD[i]);
}


if (n_sims == 0){
    GOF_min = GOF[0];
    best_params = calib_params[0];

} else {

    auto it = std::min_element(std::begin(GOF), std::end(GOF));
    int index = distance(GOF.begin(), it);
    GOF_min_run = GOF[index];
    if (GOF_min_run < GOF_min){
        GOF_min = GOF_min_run;
        best_params = calib_params[index];
    }
}
}

std::vector<double> calibrate::loadCalibData(int n_params, int n_sim, int tuning_factor) {

if(n_sim == 0){

    random_device rd;
    mt19937 gen(rd());

    for (int i = 0; i < n_params; i ++ ){
        uniform_real_distribution<> dis(multipliers[i][0], multipliers[i][1]);
        calib_params[n_sim][i] = dis(gen);
    }

} else {
    tuned_SD = tuningparam (n_sim, n_params, tuning_factor);

    for (int i = 0; i < n_params; i ++ ){
        calib_params[n_sim][i] = rnormal_trunc (best_params[i], tuned_SD[i], multipliers[i][1], multipliers[i][0]);
    }
}

return(calib_params[n_sim]);
}

double calibrate::WeightedDistance(double data, double mean, double SD) {
double distance = pow((data - mean)/(SD * 2),2);
return distance;
}

double calibrate::rnormal_trunc(double mu, double sigma, double upper, double lower) {
std::default_random_engine generator;
std::normal_distribution<double> distribution(mu, sigma);

double prob = distribution(generator);

while (prob < lower || prob > upper){
    prob = distribution(generator);
}

return(prob);
}

vector<double> calibrate::tuningparam(int n_sims, int n_param, int tuning_factor) {
vector<double> newSD;
for (int i = 0; i < n_param; i++){
    newSD.push_back (multipliers[i][2]/pow(tuning_factor,n_sims));
}

return newSD;
}
Jamie C
  • 1
  • 1
  • please provide a [mcve]. something is doing lots of copies, presumably `calib_params` which is being passed by value to RunCalibration? – Alan Birtles Mar 12 '19 at 15:01
  • Thank you for response -- I will try to simplify further, per your request. In meantime, calib_params is a vector of doubles that is a member of the calibrate class. Each time through the loop I update the vector and send it to the RunCalibration() function. What do you advise as a way to resolve this -- pass a pointer to the vector? – Jamie C Mar 12 '19 at 15:22
  • How big is the vector? Can it be passed by reference? – Alan Birtles Mar 12 '19 at 15:24
  • I see four places where a copy can occur: `auto ret_val = *calib_output;` and `return ret_val`. One in the invocation of `RunCalibration` and one in assigning its result to `calib_output->saved_output[i]`. The first two can be solved by returning a `unique_ptr`, the last two by passing a reference and using move assignment, respectively. – Botje Mar 12 '19 at 15:52
  • Thanks for these ideas. So would an improvement look like this: auto unique_ptr = make_unique<*calib_output>; – Jamie C Mar 12 '19 at 16:07
  • @Botje please see my edit above, with ongoing issues. – Jamie C Mar 12 '19 at 17:13
  • @JamieC no, `make_unique(*calib_output)` again makes a copy. Why not just make the unique_ptr up front and use that all the time without freeing it? OR you use `std::move` `make_unique(std::move(*calib_output))` and actually implement the move constructor for `calibrate` correctly). – Botje Mar 13 '19 at 06:58
  • @Botje I am not clear on how to implement the suggested changes. Would you be able to provide some example code that I could reference to make such changes? – Jamie C Mar 15 '19 at 16:39
  • @JamieC Show the definition of `calibrate` then, I'll make an answer. – Botje Mar 15 '19 at 18:36
  • Thanks @Botje, just added the definition to the above post. – Jamie C Mar 16 '19 at 21:34
  • I meant the declarations of the members, but okay. It does not look like you have anything in there that needs special attention memory-wise. – Botje Mar 18 '19 at 08:08
  • Thanks for the very useful comments and edits @Botje!! – Jamie C Mar 18 '19 at 23:29

1 Answers1

0

I improved RunCalibration as follows. Note the comments for further improvement opportunities.

using std::make_unique;
using std::unique_ptr;

void RunCalibration(calibrate &calib, ScreenStrat &strat_burnin, Inputs &tables, StateMachine &Machine, int i);

unique_ptr<calibrate> RunChain(string RunsFileName, string CurKey, string OutputFolder, string DataFolder) {

    auto tables = make_unique<Inputs>(OutputFolder, DataFolder);
    tables->loadRFG (RunsFileName, CurKey);
    tables->loadVariables ();

    int n_sims = tables->Simulations;
    int n_params = tables->Multipliers.size();
    int n_targs = tables->CalibTargs.size();

    auto strat_burnin = make_unique<ScreenStrat>(
            ScreenStrat::NoScreen, ScreenStrat::NoScreen,
            tables->ScreenStartAge, tables->ScreenStopAgeHIV,
            tables->ScreenStopAge, ScreenStrat::NoVaccine);

    auto calib_output = make_unique<calibrate>(n_sims, n_params, n_targs);

    // I don't know the type of these fields, but IF you do not modify them in
    // `RunCalibration`, consider making them `shared_ptr<vector<...>>`
    // both in `calibrate` and in `Inputs` so you can simply copy
    // the pointer instead of the full table.
    calib_output->multipliers_names = tables->MultipliersNames;
    calib_output->calib_targs_names = tables->CalibTargsNames;

    // Same applies here. If you do not modify CalibTargs, make `calib_targs` a shared_ptr
    // and only copy by pointer.
    for (int i = 0; i < n_targs; i ++){
        calib_output->calib_targs[i] = tables->CalibTargs[i][0];
        calib_output->calib_targs_SD[i] = tables->CalibTargs[i][1];
    }

    // and again...
    for (int i = 0; i < n_params; i++){
        for (int j = 0; j < 3; j++){
            calib_output->multipliers[i][j] = tables->Multipliers[i][j];
        }
    }

    auto Machine = make_unique<StateMachine>();

    for (int i = 0; i < n_sims; i++){
        RunCalibration(*calib_output, *strat_burnin, *tables, *Machine, i);
    }

    // This will return the unique_ptr without copying.
    return calib_output;
}
Botje
  • 26,269
  • 3
  • 31
  • 41