Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New hlt susy developments #6255

Merged
merged 5 commits into from Nov 15, 2014

Conversation

parbol
Copy link
Contributor

@parbol parbol commented Nov 6, 2014

This is the full suite of SUSY DQM HLT modules covering all the triggers we have proposed so far.

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 6, 2014

A new Pull Request was created by @parbol for CMSSW_7_3_X.

New hlt susy developments

It involves the following packages:

HLTriggerOffline/SUSYBSM

@nclopezo, @danduggan, @rovere, @cmsbuild, @deguio, @ojeda can you please review it and eventually sign? Thanks.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.
@nclopezo, @ktf you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 7, 2014

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 7, 2014

@jruizvar
Copy link
Contributor

jruizvar commented Nov 7, 2014

I checked the DQM output test results, in particular this file

https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6255/716/DQMTestsResults/DQMTestsResults/9/DQM_V0001_R000000001__Global__CMSSW_X_Y_Z__RECO.root

There are not histograms in the directory "DQMData/Run 1/HLT/Run summary/SUSYBSM/ ", only empty folders. Is it normal?

@parbol
Copy link
Contributor Author

parbol commented Nov 7, 2014

I'm not sure whether this is normal or not. To be honest I don't know very well how that root file was generated. Is it using an updated HLT with the latest greatest triggers?

@jruizvar
Copy link
Contributor

jruizvar commented Nov 7, 2014

I don't know either how this tests are run. My intention is not to delay this PR, it's just a matter of curiosity

@parbol
Copy link
Contributor Author

parbol commented Nov 10, 2014

Hi Jose,

There are things in this PR unchanged from the previous code that was working well and doesn't appear in that root file, so I think this is OK.

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_3_X IBs unless changes or unless it breaks tests.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_3_X IBs unless changes (tests are also fine). This pull request will be automatically merged.

cmsbuild added a commit that referenced this pull request Nov 15, 2014
@cmsbuild cmsbuild merged commit 9c5c267 into cms-sw:CMSSW_7_3_X Nov 15, 2014
@cmsbuild
Copy link
Contributor

@davidlange6
Copy link
Contributor

Seems that this PR is bringing crashes in the integration builds - Could you provide a quick fix? (I likely will back it out this change for the 0200 if no fix)

https://cmssdt.cern.ch/SDT/cgi-bin/buildlogs/slc6_amd64_gcc491/CMSSW_7_3_X_2014-11-17-0200/pyRelValMatrixLogs/run/22.0_SingleMuPt1000+SingleMuPt1000INPUT+DIGI+RECO+HARVEST/step3_SingleMuPt1000+SingleMuPt1000INPUT+DIGI+RECO+HARVEST.log

@parbol
Copy link
Contributor Author

parbol commented Nov 17, 2014

Hi David,

Is it possible to have the root file where this problem showed up? I'm running with my own root files and I cannot reproduce the error.

Pablo

@parbol
Copy link
Contributor Author

parbol commented Nov 17, 2014

Hi David,

I have found the problem but couldn't really see if it worked because I cannot reproduce the error (since I have very limited statistics and was not lucky enough to reach the error).

Let me describe the problem: the module

SUSY_HLT_MET_MUON_ER

looks for triggers with an eta cut on the muon. When in the code I do:

std::vector offlineMuons;
for(reco::MuonCollection::const_iterator muon = MuonCollection->begin(); muon != MuonCollection->end() ; ++muon) {
if(fabs(muon->eta())>etaMuonOffline_) continue;
Lepton theMuon; theMuon.pt = muon->pt(); theMuon.phi = muon->phi(); theMuon.eta = muon->eta();
offlineMuons.push_back(theMuon);
}

since I'm applying this eta cut, the size of "offlineMuons" can be actually smaller than the size of the MuonCollection (for example when MuonCollection has muons with high eta). Then later in the code I was doing operations requiring that MuonCollection was greater than 0, but then using offlineMuons again.

if((hasFiredAuxiliaryForMuonLeg || !e.isRealData()) && MuonCollection->size()>0 && pfMETCollection->begin()->et() > METOffline_ && pfHT > HTOffline_) {
if(hasFired && indexOfMatchedMuon >= 0) {
h_MuTurnOn_num-> Fill(offlineMuons.at(indexOfMatchedMuon).pt);
h_MuTurnOn_den-> Fill(offlineMuons.at(indexOfMatchedMuon).pt);
} else {
h_MuTurnOn_den-> Fill(offlineMuons.at(0).pt);
}
}
I think this was crashing because as I said MuonCollection could have more than one element, while offlineMuons not, and therefore offlineMuons.at(0).pt could fail.

I have updated the code in such a way that MuonCollection is not used anymore anywhere. So I think this should be fine. I would be pleased if you could check again (comitted new code).

Thanks,

Pablo

@davidlange6
Copy link
Contributor

Hi

Sounds promising - you will need to make a new pull request for integration. Meanwhile you can likely reproduce the error via

runTheMatrix -l 22 -i all

Cheers-
David

On Nov 17, 2014, at 8:46 PM, parbol notifications@github.com
wrote:

Hi David,

I have found the problem but couldn't really see if it worked because I cannot reproduce the error (since I have very limited statistics and was not lucky enough to reach the error).

Let me describe the problem: the module

SUSY_HLT_MET_MUON_ER

looks for triggers with an eta cut on the muon. When in the code I do:

std::vector offlineMuons;
for(reco::MuonCollection::const_iterator muon = MuonCollection->begin(); muon != MuonCollection->end() ; ++muon) {
if(fabs(muon->eta())>etaMuonOffline_) continue;
Lepton theMuon; theMuon.pt = muon->pt(); theMuon.phi = muon->phi(); theMuon.eta = muon->eta();
offlineMuons.push_back(theMuon);
}

since I'm applying this eta cut, the size of "offlineMuons" can be actually smaller than the size of the MuonCollection (for example when MuonCollection has muons with high eta). Then later in the code I was doing operations requiring that MuonCollection was greater than 0, but then using offlineMuons again.

if((hasFiredAuxiliaryForMuonLeg || !e.isRealData()) && MuonCollection->size()>0 && pfMETCollection->begin()->et() > METOffline_ && pfHT > HTOffline_) {
if(hasFired && indexOfMatchedMuon >= 0) {
h_MuTurnOn_num-> Fill(offlineMuons.at(indexOfMatchedMuon).pt);
h_MuTurnOn_den-> Fill(offlineMuons.at(indexOfMatchedMuon).pt);
} else {
h_MuTurnOn_den-> Fill(offlineMuons.at(0).pt);
}
}
I think this was crashing because as I said MuonCollection could have more than one element, while offlineMuons not, and therefore offlineMuons.at(0).pt could fail.

I have updated the code in such a way that MuonCollection is not used anymore anywhere. So I think this should be fine. I would be pleased if you could check again (comitted new code).

Thanks,

Pablo


Reply to this email directly or view it on GitHub.

@parbol
Copy link
Contributor Author

parbol commented Nov 17, 2014

Hi,

I run it and I don't get the error anymore. I have made a new PR:

#6457

Thanks for your help David,

PAblo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants