-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Me0 muons in 76 x commitv2 #11398
Me0 muons in 76 x commitv2 #11398
Conversation
Github tells me there are conflicts that must be merged. I've tried to fix it, but when I try to cms-merge-topic the branch it doesn't complain of any merge conflicts, and just gives me one warning: I'm not sure how to solve this, so I sent the PR. In the past when there is a conflict, trying merge-topic showed warnings for them, but I'm not sure where the conflict is now, any advice? |
A new Pull Request was created by @dnash86 for CMSSW_7_6_X. Me0 muons in 76 x commitv2 It involves the following packages: CommonTools/RecoAlgos @cmsbuild, @cvuosalo, @civanch, @mdhildreth, @slava77 can you please review it and eventually sign? Thanks. |
@cmsbuild please test |
The tests are being triggered in jenkins. |
@dnash86 |
@slava77 I did it in CMSSW_7_6_0_pre4. I tried to check for conflicts by doing cms-merge-topic in a recent IB, I believe it was CMSSW_7_6_X_2015-09-21-1100, and found this error but wasn't sure what to do: "error: addinfo_cache failed for path 'HLTriggerOffline/Exotica/python/analyses/hltExoticaPFHT_cff.py'" |
On 9/22/15 9:23 AM, dnash wrote:
You can ignore this error. I think your conflicts are with more recent changes (most likely 10353 you can wait until 09-22-2300 IB shows up or update to it now by a rebase git fetch official-cmssw CMSSW_7_6_X:CMSSW_7_6_X this should do the work and push the rebase into the same PR and update it
|
687a08f
to
90d5e5a
Compare
@slava77 Thanks a lot for the help with that, I made a mistake executing your instructions the first time, but it seems to be okay now (possible to be merged). |
@cmsbuild please test |
this unit test is failing in the IB as well |
David, The version that I have committed runs on the output produced from
|
+1
|
@slava77 Thanks for the "+1", I take it you're requesting that I send a PR for the changes to the config that you made separately (by itself in a PR)? |
On 10/6/15 8:07 PM, dnash86 wrote:
You can update this PR as well.
|
@civanch please check this PR |
+1 |
This pull request is fully signed and it will be integrated in one of the next CMSSW_7_6_X IBs (but tests are reportedly failing). This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @Degano, @smuzaffar |
@dnash86 Then the reco part becomes almost trivial: you just use the official reco and add the me0 modules. |
@slava77 Okay, thanks, I will look at your suggestions and try implementing them. The reason for the complex test file was ECAL related errors due to the piecemeal geometry. |
Did you mean to edit the DIGI file like so: process.load ('SimGeneral.PileupInformation.AddPileupSummary_cfi') For me if I just do that, it crashes on the digi step with: |
On 10/7/15 2:53 PM, dnash86 wrote:
Yes, I was hoping it was just that.
|
... about the addPileupInfo in the digi step: it looks like it's killed by the gem customs both in the sequence and in the event content. I have updated my branch (and dropped the "Timing" printouts to make it more quiet) |
This is the PR for dedicated ME0Muons for 76X. A previous PR was closed for us to test things before reopening, this is the same MEMuon functionality, now tested.
Most comments from the previous PR have been incorporated/addressed, moving the location of a few modules and some other changes as well. One comment regarding a dummy plane hasn't been implemented, we plan to send a PR for ME0 chambers in full reco soon as well. Naturally there might be further comments.