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

Fixes in L1T Calo packers - useful for data taking - 80x #13786

Merged
merged 3 commits into from Mar 22, 2016

Conversation

rekovic
Copy link
Contributor

@rekovic rekovic commented Mar 21, 2016

Fixed EG Unpacker and EtSums Unpacker and Emulation to agree with packer.
Now Unpacked and Re-emulated L1T decisions in HLT are in agreement.
Should use this in data taking immediately.

mulhearn and others added 2 commits March 21, 2016 09:13
…re bitwise test of sums

Conflicts:
	L1Trigger/L1TCommon/python/l1tComparisonStage2RAWvsEMU_cfi.py
@cmsbuild
Copy link
Contributor

A new Pull Request was created by @rekovic for CMSSW_8_0_X.

It involves the following packages:

EventFilter/L1TRawToDigi
L1Trigger/L1TCalorimeter
L1Trigger/L1TCommon

@cmsbuild, @mulhearn, @davidlange6 can you please review it and eventually sign? Thanks.
@Martin-Grunewald this is something you requested to watch as well.
@slava77, @Degano, @smuzaffar you are the release manager for this.

cms-bot commands are list here #13028

@rekovic
Copy link
Contributor Author

rekovic commented Mar 21, 2016

please start test

@Martin-Grunewald
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/11985/console

@Martin-Grunewald
Copy link
Contributor

@rekovic
The syntax to start jenkins testing is "please test"

@rekovic
Copy link
Contributor Author

rekovic commented Mar 21, 2016

@Martin-Grunewald It was meant as asking one of managers to trigger jenkins. I don't think I have the privileges to do this. I think currently Mike can do it for L1T.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@fwyzard
Copy link
Contributor

fwyzard commented Mar 22, 2016

@rekovic in L1Trigger/L1TCommon/python/l1tComparisonStage2RAWvsEMU_cfi.py, this PR has

    # for initial module testing compared unpacked to unpacked!!!
    egTagB     = cms.InputTag("caloStage2Digis","EGamma"),
    tauTagB    = cms.InputTag("caloStage2Digis","Tau"),
    jetTagB    = cms.InputTag("caloStage2Digis","Jet"),
    sumTagB    = cms.InputTag("caloStage2Digis","EtSum"),
    muonTagB   =  cms.InputTag("gmtStage2Digis","Muon"),
    algTagB    =  cms.InputTag("gtStage2Digis",""),

while #13767 has

    # for initial module testing compared unpacked to unpacked!!!
    egTagB     = cms.InputTag("caloStage2Digis"),
    tauTagB    = cms.InputTag("caloStage2Digis"),
    jetTagB    = cms.InputTag("caloStage2Digis"),
    sumTagB    = cms.InputTag("caloStage2Digis"),
    muonTagB   =  cms.InputTag("gmtStage2Digis",""),
    algTagB    =  cms.InputTag("gtStage2Digis",""),

Which version should be picked ?

@rekovic
Copy link
Contributor Author

rekovic commented Mar 22, 2016

@fwyzard The latter.
It is the #13767 which introduces the "L1TComparison" module, so we should stick to the python file
l1tComparisonStage2RAWvsEMU_cfi.py from that PR.

@rekovic
Copy link
Contributor Author

rekovic commented Mar 22, 2016

@fwyzard In any case, whichever PR (this #13786 or #13767) makes it in earlier, the other one will have to update the fie L1Trigger/L1TCommon/python/l1tComparisonStage2RAWvsEMU_cfi.py.
It will have no effect on code in this PR.

@fwyzard
Copy link
Contributor

fwyzard commented Mar 22, 2016

It is the #13767 which introduces the "L1TComparison" module, so we should stick to the python file
l1tComparisonStage2RAWvsEMU_cfi.py from that PR.

Then why do you use a different configuration here ?

In any case, whichever PR (this #13786 or #13767) makes it in earlier, the other one will have to update the fie L1Trigger/L1TCommon/python/l1tComparisonStage2RAWvsEMU_cfi.py.
It will have no effect on code in this PR.

But this is exactly what I am asking: once both are merged, what version of the python file should be used, to resolve the conflict ?

@rekovic
Copy link
Contributor Author

rekovic commented Mar 22, 2016

@fwyzard Because this PR has latest cherry-picks from a l1t-offline branch that super-seeds the l1t-tsg-v4 (i.e. #13767). One of the cherry picks picked up the change in this testing .py file, which is not used anywhere in CMSSW at the moment. When we merge l1t-tsg-v4 we should use the .py file version of that PR.

What am I missing? Are you saying that we should remove the .py file from this PR or anticipate the version from l1t-tsg-v4?

@fwyzard
Copy link
Contributor

fwyzard commented Mar 22, 2016

Are you saying that we should remove the .py file from this PR or anticipate the version from l1t-tsg-v4?

I'm saying that we should make sure this PR should merge cleanly on top of #13767 (or the opposite, depending which one should be merged first).
The most simple approach is indeed to remove l1tComparisonStage2RAWvsEMU_cfi.py from this PR, if it is not needed.

@cmsbuild
Copy link
Contributor

Pull request #13786 was updated. @cmsbuild, @mulhearn, @davidlange6 can you please check and sign again.

@Martin-Grunewald
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/12007/console

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@fwyzard
Copy link
Contributor

fwyzard commented Mar 22, 2016

thanks, this now merges cleanly on top of #13767 (and vice versa)

davidlange6 added a commit that referenced this pull request Mar 22, 2016
Fixes in L1T Calo packers - useful for data taking - 80x
@davidlange6 davidlange6 merged commit 9a6131c into cms-sw:CMSSW_8_0_X Mar 22, 2016
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

7 participants