Storing histograms via a map

I am having trouble trying to fill some histograms. I am able to fill the first set of histograms (DiLeptMass) but it crashes when I then try to fill the DiLeptLifetime histograms.

I have issue when filling, i.e the programme crashes at hist->Fill(Iter2->second.at(0),1);

I’ve included the relevant parts of the code.

 std::map< TString, TH1* > myMap;   //map to store the histograms
std::map< RunEvt, std::vector<Double_t> > MapOfEventsAndMasses //Map of Events and      DiLept Masses
 std::map < RunEvt, std::vector<Double_t> > MapOfEventsAndLifetimes; //map of Events and DiLept Lifetimes

  for( Int_t i=0; i<5; i++){
    
        for(Int_t j=0; j<4; j++){
        
       TString myString = Form("DiLeptMass_%s%i%i", "DiLeptMass", i, j);
           TH1D *hist = new TH1D(myString, "test_mass_plot", 80.0, mass_low[i], mass_high[i]);
           myMap[myString] = hist;
    // cout << myString << " " << myMap[myString] << endl;

        myString = Form("DiLeptLifetimes_%s%i%i", "DileptLifetimes", i , j);
        hist = new TH1D (myString, "test_lifetime_plot", 200.0, lifetime_low[j], lifetime_high[j]);
        myMap[myString]= hist;
        //cout << myString << " " << myMap[myString] << endl;
  }
}


    //##################################
        //    CREATE MAPS OF THE VARIABLES
       //##################################

        if(MapOfEventsAndMasses.find(my_event) == MapOfEventsAndMasses.end()){  //change event to my_event
           std::vector<Double_t> tempVec;
           tempVec.push_back(DiLept_M);
           MapOfEventsAndMasses[my_event] = tempVec;
         }else{
          MapOfEventsAndMasses[my_event].push_back(DiLept_M);
        }


 if(MapOfEventsAndLifetimes.find(my_event) == MapOfEventsAndLifetimes.end()){
        
         std::vector<Double_t> lifetimeVec;  //Declaring a vector to hold Photon pTs
        
         lifetimeVec.push_back(DiMuonVertex_Tau); //If the index exists, acess the second index of the map, and using push back load the Photon_Pt into the vector
        
         MapOfEventsAndLifetimes[my_event] = lifetimeVec;
        
       }
      else{
          MapOfEventsAndLifetimes[my_event].push_back(DiMuonVertex_Tau);
       }
    

    //#######################
    /// DECLARE ITERATORS
    // #####################
   std::map < RunEvt, std::vector <Double_t> >::iterator Iter1 =    MapOfEventsAndMasses.begin();
    std::map < RunEvt, std::vector <Double_t> >::iterator Iter1_End = MapOfEventsAndMasses.end();
   
   std::map < RunEvt, std::vector <Double_t> >::iterator Iter2 = MapOfEventsAndLifetimes.begin();
   std::map < RunEvt, std::vector <Double_t> >::iterator Iter2_End = MapOfEventsAndLifetimes.end();


 //#######################################
 //  Iterate through the variables and fill histograms
//########################################
 while(Iter1 != Iter1_End){

     if(Iter1->second.size()==1 && Iter2->second.size()==1){
    
        for (Int_t i=0; i<5; ++i) {
        
       
             for(Int_t j=0; j<4; ++j){
            
            
           if (Iter1->second[0] < mass_low[i] || Iter1->second[0] > mass_high[i]) continue; // failed cut

	              
	   Iter2 = MapOfEventsAndLifetimes.find(Iter1->first);
	
            
                if (Iter2->second[0] < lifetime_low[j] || Iter2->second[0] > lifetime_high[j]) continue; // failed cut

       	
	
           TString myString = Form("DiLeptMass_%s%i%i", "DiLeptMass", i, j);
               auto hist = myMap[myString];
               //cout << "DEBUG: " << myString << " " << hist << "  " << Iter1->second.size() << endl;
               assert(hist);
               hist->Fill(Iter1->second.at(0),1);

        		
	   myString = Form("DiLeptLifetimes_%s%i%i", "DiLeptLifetimes", i, j);	        
	   hist = myMap[myString];	        
	   //cout << "DEBUG: " << myString << " " << hist << "  " << Iter2->second.size() << endl;
	   //assert(hist);
         	hist->Fill(Iter2->second.at(0),1);
	
    }
  }
   }

It’s hard in this code to find the problem or to check if it is correct or not.

Some remarks to simplify the code and find the problem:

a) The map’s operator[] creates elements if they don’t exist. Therefore the following checks for existance are useless:

if (MapOfEventsAndMasses.find(my_event)
    == MapOfEventsAndMasses.end()) { // change event to my_event
    std::vector<Double_t> tempVec;
    tempVec.push_back(DiLept_M);
    MapOfEventsAndMasses[my_event] = tempVec;
} else {
    MapOfEventsAndMasses[my_event].push_back(DiLept_M);
}
if (MapOfEventsAndLifetimes.find(my_event) == MapOfEventsAndLifetimes.end()) {

    std::vector<Double_t> lifetimeVec; // Declaring a vector to hold Photon pTs

    lifetimeVec.push_back(DiMuonVertex_Tau); // If the index exists, acess the second index of
                                             // the map, and using push back load the Photon_Pt
                                             // into the vector

    MapOfEventsAndLifetimes[my_event] = lifetimeVec;

} else {
    MapOfEventsAndLifetimes[my_event].push_back(DiMuonVertex_Tau);
}

This looong code can be simplified to:

    MapOfEventsAndMasses[my_event].push_back(DiLept_M);
    MapOfEventsAndLifetimes[my_event].push_back(DiMuonVertex_Tau);

In your big loop with the two iterators, I would suggest two things.
a)

                auto hist = myMap[myString];
                // cout << "DEBUG: " << myString << " " << hist << "  " << Iter1->second.size()
                // << endl;
                assert(hist);

You want to make sure the histogram exists, but you don’t want to MODIFY the map! Question: are assertions enabled? If not, you might not see the problem. What you really want is the following:

                auto hist = myMap.at(myString);

With .at instead of operator[] you ensure that the element exists and that the map isn’t modified!

Also in the loop, you could replace all [] with .at to add bounds checking.

As I find it hard to understand your logic with your two iterators (what is that function doing / supposed to be doing?), I would suggest to modify your data structure. Instead of two maps with vectors like this:

std::map<RunEvt, std::vector<Double_t>>
    MapOfEventsAndMasses; // Map of Events and      DiLept Masses
std::map<RunEvt, std::vector<Double_t>>
    MapOfEventsAndLifetimes; // map of Events and DiLept Lifetimes

I would recommend to use one map of a struct:

struct MyEventData { // find a better name for this struct
  std::vector<Double_t> masses;
  std::vector<Double_t> liefetimes;
};
std::map<RunEvt, MyEventData> mapEvtToMassAndLifetime;

or

struct ParticleProperty {
  Double_t mass;
  Double_t liefetime;
};
std::map<RunEvt, std::vector<ParticleProperty>> mapEvtToMassAndLifetime;

Whatever seems more useful.

Hey,

Thanks for your very in depth reply, I am certainly grateful. I have made a few of the changes (I’ve not altered the maps yet), but when I replaced all the [] with .at(0) in the two for loops, and made the following change: auto hist = myMap.at(myString); I then got the following error: terminate called after throwing an instance of 'std::out_of_range’
what(): map::at

So that means the entry doesn’t exist. So as for debugging, add a cout << myString << '\n'; just before the map.at() line to find out which histogram does not exist.

This should have triggered the assertion as well (did you run your old script with .x script.C+g ? Without the g, the assertion check is optimized out)

Hey,

So at this point they seem to exist:

for( Int_t i=0; i<5; i++){
    
    for(Int_t j=0; j<4; j++){
        
    TString myString = Form("DiLeptMass_%s%i%i", "DiLeptMass", i, j);
        TH1D *hist = new TH1D(myString, "test_mass_plot", 80.0, mass_low[i], mass_high[i]);
        myMap[myString] = hist;
    cout << myString << " " << myMap[myString] << endl;

    myString = Form("DiLeptLifetimes_%s%i%i", "DileptLifetimes", i , j);
    hist = new TH1D (myString, "test_lifetime_plot", 200.0, lifetime_low[j], lifetime_high[j]);
    myMap[myString]= hist;
    cout << myString << " " << myMap[myString] << endl;

  }
 }

But then at this point they do not:

while(Iter1 != Iter1_End){

  if(Iter1->second.size()==1 && Iter2->second.size()==1){
    
    for (Int_t i=0; i<5; i++) {
        
       
        for(Int_t j=0; j<4; j++){
            
            
      if (Iter1->second.at(0) < mass_low[i] || Iter1->second.at(i) > mass_high[i]) continue; // failed cut

	              
	Iter2 = MapOfEventsAndLifetimes.find(Iter1->first);
	
            
            if (Iter2->second.at(0) < lifetime_low[j] || Iter2->second.at(0) > lifetime_high[j]) continue; // failed cut

       	
	
        TString myString = Form("DiLeptMass_%s%i%i", "DiLeptMass", i, j);
	cout << myString << '\n';
            auto hist = myMap.at(myString);
            //cout << "DEBUG: " << myString << " " << hist << "  " << Iter1->second.size() << endl;
            assert(hist);
	// hist->Fill(Iter1->second.at(0),1);

        		
	myString = Form("DiLeptLifetimes_%s%i%i", "DiLeptLifetimes", i, j);
	cout << myString << '\n';
	hist = myMap.at(myString);	        
	//cout << "DEBUG: " << myString << " " << hist << "  " << Iter2->second.size() << endl;
	assert(hist);
       	//hist->Fill(Iter2->second.at(0),1);
	
  }
 }
}

}

Are you modifying myMap?
You could also add the following debug output:

cout << "myMap contains the following keys: ";
for (const auto &p : myMap) cout << p.first << " ";
cout << "\n";

Or use a debugger :slight_smile:

Edit: I’ve spotted the bug:
it is DiLeptLifetimes vs. DileptLifetimes. Use a constant for this to avoid such problems!

Hey,

Thank you for all your help. It is much appreciated. I am going to implement the new map structure to make life easier. Thank you again.