parameter storage constant factor speed improvements?

[tdhock]

in bcef044 there was a change to the storage method for the segment-specific parameters (mean, var). Can we get speedups back by moving the storage back to class members?

[tdhock]

actually bcef044 did introduce the unordered_map but did not result in slowdowns,

figure shows four different versions from the l1loss PR https://github.com/tdhock/binsegRcpp/pull/10/commits with the slowdown happening after the mvl_construct commit (which is still fast). So probably in the New Distribution methods commit 544a4df which deletes the following

void Set::set_mvl(int first, int last, MeanVarLoss *MVL){
  set_mean_var_loss
    (first, last,
     &(MVL->param_map["mean"]),
     &(MVL->param_map["var"]),
     &(MVL->loss));
}

and adds the following

 MeanVarLoss estimate_params(Set &subtrain, int first, int last){
    subtrain.set_totals(first, last);
    MeanVarLoss mvl;
    mvl.param_map["mean"] =
      subtrain.total_weighted_data/subtrain.total_weights;
    mvl.param_map["var"]  =
      subtrain.total_weighted_squares/subtrain.total_weights +
      mvl.param_map["mean"]*
      (mvl.param_map["mean"]-
       2*subtrain.total_weighted_data/subtrain.total_weights);
    mvl.loss = compute_loss
      (subtrain.total_weights,
       subtrain.total_weighted_data,
       subtrain.total_weighted_squares,
       mvl.param_map["mean"],
       mvl.param_map["var"],
       subtrain.max_zero_var);
    return mvl;
  }

so maybe the slowdown is a result of the new copy induced by the MeanVarLoss return type (vs passing a pointer as a function arg previously).

[tdhock]

there is also the get_best_split method which returns a Split, and calls the Split constructor in each iteration of this for loop

    Split best_split;
    for(int candidate=first_candidate; candidate<=last_candidate; candidate++){
      Split candidate_split;
      candidate_split.this_end = candidate;
      candidate_split.before = estimate_params
        (subtrain, first_data, candidate);
      candidate_split.after = estimate_params
        (subtrain, candidate+1, last_data);
      best_split.maybe_update(candidate_split);
    }

[tdhock]

this code was added in commit that resulted in slowdown,

  void maybe_update(Split &candidate) {
    if(candidate.get_loss() < get_loss()){
      Rprintf("loss old=%f new=%d\n", get_loss(), candidate.get_loss());
      *this = candidate;
    }
  }

that results in copy of the Split instance every time there is a better candidate found. Before we did the following,

  for(int candidate=first_candidate; candidate<=last_candidate; candidate++){
    loss_split = candidate_split.get_split_loss
      (subtrain, first_data, candidate, last_data);
    if(loss_split < best_loss_split){
      best_loss_split = loss_split;
      best_split = candidate_split;
    }
  }

both versions above result in copying the Split.
new/slow

class Split {
public:
  int this_end;//index of last data point on the first/before segment.
  MeanVarLoss before, after;
  double get_loss(void) const {
    return before.loss + after.loss;
  }
  void maybe_update(Split &candidate) {
    if(candidate.get_loss() < get_loss()){
      Rprintf("loss old=%f new=%d\n", get_loss(), candidate.get_loss());
      *this = candidate;
    }
  }
};

old/fast

class Split {
public:
  int this_end;//index of last data point on the first/before segment.
  MeanVarLoss before, after;
  double get_split_loss(Set &subtrain, int first, int end_i, int last);
};