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

Fix PR #6576 (Me0 global reco) #6585

Merged
merged 4 commits into from Nov 25, 2014
Merged

Conversation

mark-grimes
Copy link

Pull request #6576 switched from using EmulatedME0Segment to ME0Segment. This just updates

FastSimulation/Configuration/test/TestAnalyzer_Final.cc

for the change. Note that I only fixed the compiler error, I haven't done any testing yet. I'll run the standard upgrade matrix tests after this has been submitted, but they won't test TestAnalyzer_Final.cc.

@dnash86

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @mark-grimes (Mark Grimes) for CMSSW_6_2_X_SLHC.

Fix PR #6576 (Me0 global reco)

It involves the following packages:

DataFormats/MuonReco
FastSimulation/Configuration
RecoMuon/MuonIdentification
SLHCUpgradeSimulations/Configuration
Validation/RecoMuon

@civanch, @nclopezo, @StoyanStoynev, @lveldere, @danduggan, @mdhildreth, @cmsbuild, @deguio, @slava77 can you please review it and eventually sign? Thanks.
@battibass, @abbiendi, @bellan, @trocino, @amagitte, @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.
@fratnikov, @mark-grimes you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@mark-grimes mark-grimes mentioned this pull request Nov 25, 2014
@@ -50,13 +52,11 @@ void ME0MuonConverter::produce(edm::Event& ev, const edm::EventSetup& setup) {
using namespace reco;

Handle <std::vector<ME0Muon> > OurMuons;
ev.getByLabel <std::vector<ME0Muon> > ("me0SegmentMatcher", OurMuons);

ev.getByLabel <std::vector<ME0Muon> > ("me0SegmentMatching", OurMuons);
Copy link
Author

Choose a reason for hiding this comment

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

@dnash86 Should there also be a change to the default config/customisation to match this change?
I get a failure in Reco for many workflows (e.g. 13800):

---- Begin Fatal Exception 25-Nov-2014 13:07:29 CET-----------------------
An exception of category 'ProductNotFound' occurred while
[0] Processing run: 1 lumi: 1 event: 1
[1] Running path 'reconstruction_step'
[2] Calling event method for module ME0MuonConverter/'me0MuonConverter'
Exception Message:
Principal::getByLabel: Found zero products matching all criteria
Looking for type: std::vector<reco::ME0Muon>
Looking for module label: me0SegmentMatching
Looking for productInstanceName:

As far as I can see there is no python change to switch the name of the producer. Also might be an idea to have this as a config parameter.

Copy link
Contributor

Choose a reason for hiding this comment

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

@mark-grimes Yes, it seems I didn't add the python files I had changed in my local area to the commit, my mistake. I tested 13800 locally (as well as 13007, earlier), and it works. I've added a new commit with the 3 needed python configs, but I think they are still on PR #6576. How can I merge it with this PR?

Copy link
Author

Choose a reason for hiding this comment

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

I'll grab the commit and apply directly myself.

For future reference though, it's possible to put in a pull request directly on to my branch (mark-grimes:fix6576) and then the pull request gets updated. I could have done the same onto your branch and modify #6576 directly, but it requires you to accept it - in the past I've been waiting around for people to accept, so I find it easier to put in a new request myself.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I see. Thanks a lot for the information about future pull requests. I'll keep that in mind.

@mark-grimes
Copy link
Author

merge

@cmsbuild
Copy link
Contributor

Pull request #6585 was updated. @civanch, @nclopezo, @StoyanStoynev, @lveldere, @danduggan, @mdhildreth, @cmsbuild, @deguio, @slava77 can you please check and sign again.

cmsbuild added a commit that referenced this pull request Nov 25, 2014
@cmsbuild cmsbuild merged commit 37c9d37 into cms-sw:CMSSW_6_2_X_SLHC Nov 25, 2014
@mark-grimes mark-grimes deleted the fix6576 branch November 28, 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

3 participants