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

DQM JetMET from cmssw 5_3_12_patch2 #1929

Merged
merged 4 commits into from
Feb 5, 2014
Merged

DQM JetMET from cmssw 5_3_12_patch2 #1929

merged 4 commits into from
Feb 5, 2014

Conversation

BetterWang
Copy link
Contributor

DQM JetMET for HeavyIon physics from cmssw 5_3_12_patch2
Contact person:
Quan Wang, quan.wang@cern.ch,
Pelin Kurt, pelin.kurt@cern.ch,
Matthew Nguyen, Matthew.Nguyen@cern.ch,
Yetkin Yilmaz, yetkin.yilmaz@cern.ch

Quan Wang added 2 commits December 30, 2013 00:34
Doesn't work for workflow 40.0 step3.
----- Begin Fatal Exception 30-Dec-2013 00:38:40 CET-----------------------
An exception of category 'PluginNotFound' occurred while
   [0] Constructing the EventProcessor
Exception Message:
Unable to find plugin 'CaloJetTesterUnCorr_HeavyIons'. Please check spelling of name.
----- End Fatal Exception -------------------------------------------------
@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 1, 2014

A new Pull Request was created by @BetterWang for CMSSW_5_3_X.

DQM JetMET from cmssw 5_3_12_patch2

It involves the following packages:

Validation/RecoHI
Validation/RecoJets

@nclopezo, @danduggan, @rovere, @cmsbuild, @anton-a, @thspeer, @deguio, @slava77, @eliasron can you please review it and eventually sign? Thanks.
@kkrajczar, @TaiSakuma, @RylanC24, @richard-cms this is something you requested to watch as well.
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.
@smuzaffar you are the release manager for this.

#+ (iterativeCone7HiCleanedGenJets * JetAnalyzerICPU7Calo)
#+ (ak5HiCleanedGenJets * JetAnalyzerAkPU5Calo * JetAnalyzerAkFastPU5Calo
#+ (ak7HiCleanedGenJets*JetAnalyzerAkPU7Calo *JetAnalyzerAkFastPU7Calo)
ak3HiCleanedGenJets
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BetterWang
it looks like the producer ak3HiCleanedGenJets is called two times here. could you please fix it?

@deguio
Copy link
Contributor

deguio commented Jan 9, 2014

ciao @BetterWang
the analysers CaloJetTesterUnCorr and CaloJetTesterUnCorr_HeavyIons are very similar. Isn't there a way to use the same code instead of duplicating it? this is in general a good criterion to avoid to maintain copies of the same code.

could you also fix the double call to the ak3HiCleanedGenJets module?
thanks,
F.

@BetterWang
Copy link
Contributor Author

Hi @deguio ,
I can fix the double call, and work with the developer.
Do you want me the send another pull request, or just modify this one (don't know if it is possible)?

…nJets and one ak5HiCleanedGenJets.

Pelin Kurt <Pelin.Kurt@cern.ch>
@pkurt
Copy link

pkurt commented Jan 9, 2014

Hi deguio,

It is true that the our analyzer name "CaloJetTesterUnCorr_HeavyIons" is very similar to the other one "CaloJetTesterUnCorr". But they are not identical.
We wanted to have our own analyzer in order to include some performance checks for the bkg subtraction and some other interested observables in our reco. We have included one new plot so far for this purpose but we will add more soon. Hope it is OK for you as well. I have informed JetMETDQM contacts (Robert, Matthias and Viola) about this change already.
Pelin

@deguio
Copy link
Contributor

deguio commented Jan 9, 2014

@pkurt @BetterWang
if it is needed and if you think that the code will diverge even more in the future then it is ok.
I will wait for the double call fix. just commit in the same branch.

I will ask you to report during one of the next DQM meetings to show the recent developments and the plans for the future. I will contact you by email.

thanks,
F.

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 9, 2014

Pull request #1929 was updated. @thspeer, @danduggan, @rovere, @cmsbuild, @anton-a, @nclopezo, @deguio, @slava77, @Degano, @ojeda can you please check and sign again.

@pkurt
Copy link

pkurt commented Jan 9, 2014

Sure, thank you!

Pelin

@deguio
Copy link
Contributor

deguio commented Jan 9, 2014

@BetterWang @pkurt
the collection produced by JetAnalyzerICPU5Calo (the one you have just added) is not used anywhere apparently. all the analysers which take it in input are commented in the sequence. Am I wrong? if this is the case I would comment it out so that we don't produce useless collections.

thanks,
F.

@pkurt
Copy link

pkurt commented Jan 9, 2014

You are right, we will fix it shortly.
Pelin

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 9, 2014

Pull request #1929 was updated. @thspeer, @danduggan, @rovere, @cmsbuild, @anton-a, @nclopezo, @deguio, @slava77, @Degano, @ojeda can you please check and sign again.

@deguio
Copy link
Contributor

deguio commented Jan 9, 2014

+1

@BetterWang
Copy link
Contributor Author

Can we have this pull request checked and signed?

Thanks,
Quan

@davidlange6
Copy link
Contributor

@ktf - why does this need a reco signature?

@ktf
Copy link
Contributor

ktf commented Feb 3, 2014

Historically Validation/Reco* did. Shall we drop it?

@davidlange6
Copy link
Contributor

I'll let the experts comment… I'm surprised though.

On Feb 3, 2014, at 12:35 PM, Giulio Eulisse notifications@github.com
wrote:

Historically Validation/Reco* did. Shall we drop it?


Reply to this email directly or view it on GitHub.

@ktf
Copy link
Contributor

ktf commented Feb 3, 2014

I'll let the experts comment… I'm surprised though.

I think this dates back to when you were the expert… ;-) Anyways…
@slava77, @anton-a do you still want to have sign off rights to
Validation/Reco*?

@slava77
Copy link
Contributor

slava77 commented Feb 3, 2014

I don't have strong feelings about this one.
This is downstream from what we do
I'd rather have more (or some) sign off rights for other things that affect RECO physics performance (like geometry or calibrations)

@BetterWang
Copy link
Contributor Author

Then, who should sign this one off, if there is no obvious reason to keep it pending?

@davidlange6
Copy link
Contributor

+1

@smuzaffar
Copy link
Contributor

+tested

smuzaffar added a commit that referenced this pull request Feb 5, 2014
…_patch2

DQM JetMET from cmssw 5_3_12_patch2
@smuzaffar smuzaffar merged commit 2259fdd into cms-sw:CMSSW_5_3_X Feb 5, 2014
@BetterWang BetterWang deleted the DQM_JetMET_from-CMSSW_5_3_12_patch2 branch February 6, 2014 12:23
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

8 participants