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 for Tag&Probe in 7.4.X (and later) #8801

Closed
wants to merge 8 commits into from

Conversation

gpetruc
Copy link
Contributor

@gpetruc gpetruc commented Apr 20, 2015

Fix a couple of issues needed to get tag and probe running in 7.4.X release.
Tested for muons using 74X https://github.com/cms-analysis/MuonAnalysis-TagAndProbe (after PR 2)
Not terribly clear whether it's ok also from electron side

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @gpetruc (Giovanni Petrucciani) for CMSSW_7_5_X.

Fixes for Tag&Probe in 7.4.X (and later)

It involves the following packages:

DataFormats/PatCandidates
MuonAnalysis/MuonAssociators
PhysicsTools/TagAndProbe
RecoMuon/MuonIsolationProducers

@nclopezo, @cvuosalo, @monttj, @cmsbuild, @slava77, @vadler can you please review it and eventually sign? Thanks.
@battibass, @abbiendi, @jhgoh, @bellan, @trocino, @bachtis, @rociovilar 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.
If you are a L2 or a release manager you can ask for tests by saying 'please test' in the first line of a comment.
@nclopezo you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@slava77
Copy link
Contributor

slava77 commented Apr 20, 2015

@cmsbuild please test

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

-1
Tested at: a61a226
I found a compilation error while trying to compile with clang:
I used this command:
scram b vclean && scram build -k -j 8 USER_CXXFLAGS='-fsyntax-only' COMPILER='llvm compile'

>> Compiling edm plugin /build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_7_5_X_2015-04-20-1100/src/ElectroWeakAnalysis/ZMuMu/plugins/DebugZMCTruth.cc 
warning: unknown warning option '-Wno-psabi' [-Wunknown-warning-option]
In file included from /build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_7_5_X_2015-04-20-1100/src/ElectroWeakAnalysis/ZMuMu/bin/zFitToyMc.cpp:11:
In file included from /afs/cern.ch/cms/sw/ReleaseCandidates/vol1/slc6_amd64_gcc491/cms/cmssw-patch/CMSSW_7_5_X_2015-04-20-1100/src/PhysicsTools/Utilities/interface/RootMinuit.h:12:
In file included from /afs/cern.ch/cms/sw/ReleaseCandidates/vol1/slc6_amd64_gcc491/lcg/root/6.02.00-eccfad/include/Math/SMatrix.h:88:
/afs/cern.ch/cms/sw/ReleaseCandidates/vol1/slc6_amd64_gcc491/lcg/root/6.02.00-eccfad/include/Math/MatrixRepresentationsStatic.h:162:24: fatal error: recursive template instantiation exceeded maximum depth of 256
      typedef typename make_indices_impl,
                       ^
/afs/cern.ch/cms/sw/ReleaseCandidates/vol1/slc6_amd64_gcc491/lcg/root/6.02.00-eccfad/include/Math/MatrixRepresentationsStatic.h:162:24: note: in instantiation of template class 'ROOT::Math::rowOffsetsUtils::make_indices_impl<251, ROOT::Math::rowOffsetsUtils::indices<0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84, 85, 86, 87, 88, 89, 90, 91, 92, 93, 94, 95, 96, 97, 98, 99, 100, 101, 102, 103, 104, 105, 106, 107, 108, 109, 110, 111, 112, 113, 114, 115, 116, 117, 118, 119, 120, 121, 122, 123, 124, 125, 126, 127, 128, 129, 130, 131, 132, 133, 134, 135, 136, 137, 138, 139, 140, 141, 142, 143, 144, 145, 146, 147, 148, 149, 150, 151, 152, 153, 154, 155, 156, 157, 158, 159, 160, 161, 162, 163, 164, 165, 166, 167, 168, 169, 170, 171, 172, 173, 174, 175, 176, 177, 178, 179, 180, 181, 182, 183, 184, 185, 186, 187, 188, 189, 190, 191, 192, 193, 194, 195, 196, 197, 198, 199, 200, 201, 202, 203, 204, 205, 206, 207, 208, 209, 210, 211, 212, 213, 214, 215, 216, 217, 218, 219, 220, 221, 222, 223, 224, 225, 226, 227, 228, 229, 230, 231, 232, 233, 234, 235, 236, 237, 238, 239, 240, 241, 242, 243, 244, 245, 246, 247, 248, 249, 250>, 289>' requested here
      typedef typename make_indices_impl,
                       ^


you can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8801/3353/summary.html

@slava77
Copy link
Contributor

slava77 commented Apr 23, 2015

+1

for #8801 98cd7c2
the last changes have affected only PhysicsTools/TagAndProbe, not affecting the earlier reco signature for #8801 a61a226

I double-checked that the jenkins tests comparisons show no differences as expected

@cmsbuild
Copy link
Contributor

-1
@gpetruc This pull request cannot be automatically merged, could you please rebase it?
you can see the log for git cms-merge-topic here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8801/5162/git-merge-result

@slava77
Copy link
Contributor

slava77 commented Jun 2, 2015

@gpetruc please comment if we should expect a fix with the merge conflict here soon.
Thank you.
[we also need to figure out how to not lose PRs for this long]

@gpetruc
Copy link
Contributor Author

gpetruc commented Jun 2, 2015

Hi Slava,

I will do, but probably by the end of the week.

Giovanni

On Tue, Jun 2, 2015 at 3:54 PM, Slava Krutelyov notifications@github.com
wrote:

@gpetruc https://github.com/gpetruc please comment if we should expect
a fix with the merge conflict here soon.
Thank you.
[we also need to figure out how to not lose PRs for this long]


Reply to this email directly or view it on GitHub
#8801 (comment).

@gpetruc
Copy link
Contributor Author

gpetruc commented Jun 8, 2015

I would rather not push the successful merge to this same branch since this one currently merges fine in 74X while the one fixing the merge conflict in 75X will then by design merge only in 75X.
Unless you have strong objections, I'll then close this and open a new PR from the new branch.

@slava77
Copy link
Contributor

slava77 commented Jun 8, 2015

@gpetruc Giovanni, pick what's the easiest for you.

@gpetruc
Copy link
Contributor Author

gpetruc commented Jun 8, 2015

Ok, it's now in #9512

@gpetruc gpetruc closed this Jun 8, 2015
cmsbuild added a commit that referenced this pull request Jun 12, 2015
Fixes for Tag&Probe in 7.5.X (fix of #8801)
@gpetruc gpetruc deleted the TnP-74X-Bare branch March 8, 2021 09:10
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

4 participants