0

I need to erase elements from a vector based on their value, I've tried the idiom erase-remove_if but the condition for the item to be erased is not so simple, I've tried something like

map<string, TLorentzVector> map_jets
vector<pair<string,double> > jets_pt


  for( vector<pair<string,double> >::iterator it1 = jets_pt.begin(); it1 != jets_pt.end(); ){
   if( fabs(map_jets[it1->first].PseudoRapidity()) > 2.5 )
      jets_pt.erase(it1);

but I get a segmentation violation when jets_pt has size = 1. The whole program takes data of an experiment and loops over, the map keeps track of the name of the event and the associated variable I need, while the vector stores the string of the map and a value that I need to keep track of. I want to delete from the vector those value which does'n satisfy a few condition

  if( fabs(map_jets[it1->first].PseudoRapidity()) > 2.5 )
      jets_pt.erase(it1);

    if( map_jets[it->first].DeltaR(map_leps["lep1"]) < 0.4 && map_jets[it->first].DeltaR(map_leps["lep2"]) < 0.4 && map_jets[it->first].DeltaR(map_leps["lep3"]) )                                            
        jets_pt.erase(it);  

     if( jets_emfr[k] > 0.9 )                                                                                                                                                                                  
        jets_pt.erase(it);
drkg4b
  • 377
  • 3
  • 14

4 Answers4

2

This sounds like a job for the std::remove_if algorithm, which can be used to remove elements matching a given predicate from a container.

Something like this ought to work:

bool myPredicate( pair<string, double> element ) {
  return fabs(map_jets[elem.first].PseudoRapidity()) > 2.5
         || (map_jets[elem.first].DeltaR(map_leps["lep1"]) < 0.4 && ... )    
         || ...
}

jets_pt.erase( remove_if( jets_pt.begin(), jets_pt.end(), myPredicate ),
               jets_pt.end() );
Frerich Raabe
  • 90,689
  • 19
  • 115
  • 207
  • I've tried it creating a function `bool eta(map map_jets, string jet){ return( fabs(map_jets[jet].PseudoRapidity()) > 2.5 );` but for the `remove_it` the function needs to take only one argument of the same tipe of the container. – drkg4b Nov 24 '11 at 20:07
  • Yes, but you should declare map_jets as an argument of the function. – drkg4b Nov 24 '11 at 20:12
  • Use a functor to store a reference to the `map_jets` map and any other variables you need, such as `k`. See http://stackoverflow.com/questions/356950/c-functors-and-their-uses – Collin Dauphinee Nov 24 '11 at 20:13
1

I assume in your real code you actually call operator++ for the iterator somewhere in the loop. However you still have the problem that erase invalidates the iterator, so you would need to do

it = jets_pt.erase(it1);

However the remove_if - erase is really a much more suitable solution here I ould remmend something like the following:

struct remove_functor
{
    map<string, TLorentzVector>& map_jets;
    map<...>& map_leps;

    remove_functor(map<string, TLorentzVector>& m_jets, map<...>& m_leps): map_jets(m_jets), map_leps(m_leps)
    {}

    bool operator()(const pair<string,double>& p)
    {
        return (fabs(map_jets[p.first].PseudoRapidity()) > 2.5) 
           ||  ((map_jets[p.first].DeltaR(map_leps["lep1"]) < 0.4)
             && (map_jets[p.first].DeltaR(map_leps["lep2"]) < 0.4)
             && (map_jets[p.first].DeltaR(map_leps["lep3"]));
    }
 }

Then you can simply use

jets_pt.erase(remove_if(jets_pt.begin(), jets_pt.end(), remove_functor(map_jets, map_leps)), jets_pt.end());

Of course in c++0x you can simply use a lambda function:

auto predicate = [&](const pair<string, double>& p) {bool operator()(const pair<string,double>& p)
    {
        return (fabs(map_jets[p.first].PseudoRapidity()) > 2.5) 
           ||  ((map_jets[p.first].DeltaR(map_leps["lep1"]) < 0.4)
             && (map_jets[p.first].DeltaR(map_leps["lep2"]) < 0.4)
             && (map_jets[p.first].DeltaR(map_leps["lep3"]));
    };
jets_pt.erase(remove_if(jets_pt.begin(), jets_pt.end(), predicate), jets_pt.end());

predicate = [&](...){...}; creates an functor (by means of lambda syntax), which captures all used variables by reference (so map_jets, map_leps,..., indicated by the [&]) which can be used for the remove_if. auto means that the compiler should infer the type of the variable (since we don't have a name for the type generated by the compiler for this lambda).

Grizzly
  • 19,595
  • 4
  • 60
  • 78
  • Ok it seems that I have to learn to use objects! I'm just a physicist learnig to program in c++ to work, thanks. – drkg4b Nov 24 '11 at 20:17
  • Ok, I've tried the code and get a segmentation violation when `jets_pt.size() = 1` I don't see why. – drkg4b Nov 25 '11 at 19:26
  • @drkg4b: Could you give some more informations about where the segfault happens, are you sure it's a vector access and not somewhere in the map accesses and does in segfault if the element is removed or if it isn't or both? – Grizzly Nov 27 '11 at 11:28
1

Try the following (approximative c++11, I don't have access to a c++11 conforming compiler on this computer):

jets_pt.erase(
  remove_if(
    jets_pt.begin(),
    jets_pt.end(),
    [&](const pair<string,double> &p) {
      return fabs(map_jets[p.first].PseudoRapidity()) > 2.5;
    }),
  jets_pt.end());

Basically, you want to erase from the iterator returned by remove_if up to the end of the vector. You may edit the predicate to remove_if as needed.

ZeRemz
  • 1,770
  • 1
  • 13
  • 16
  • What does the `[&]` do? I mean, why in the square brakets? – drkg4b Nov 24 '11 at 20:22
  • @drkg4b: That's the new lambda syntax in c++11. It basically says that the generated functor should capture all used variables from the encolsing scope by reference (so map_jets, ...) – Grizzly Nov 24 '11 at 20:29
  • Ok, I don't think I have a c++11 compiler, this think is to be compiled by CINT of the CERN ROOT data analyzing program. – drkg4b Nov 24 '11 at 20:33
0

Do not erase entries while iterating! You invalidate your loop constraints.

Will Bickford
  • 5,381
  • 2
  • 30
  • 45