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

added consumes interface to RecoTau sequence #1718

Merged

Conversation

jpavel
Copy link
Contributor

@jpavel jpavel commented Dec 7, 2013

No description provided.

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 7, 2013

A new Pull Request was created by @jpavel (Pavel Jez) for CMSSW_7_0_X.

added consumes interface to RecoTau sequence

It involves the following packages:

RecoTauTag/TauTagTools
RecoTauTag/RecoTau

@nclopezo, @cmsbuild, @thspeer, @slava77 can you please review it and eventually sign? Thanks.
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.
@ktf you are the release manager for this.

@thspeer
Copy link
Contributor

thspeer commented Dec 9, 2013

working @thspeer

flightPathSig = iConfig.getParameter<double>("flightPathSig");
withPVError = iConfig.getParameter<bool>("UsePVerror");
booleanOutput = iConfig.getParameter<bool>("BooleanOutput");
edm::ConsumesCollector iC(consumesCollector());
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not needed. Just do

vertexAssociator_ = new reco::tau::RecoTauVertexAssociator(iConfig.getParameter("qualityCuts"), consumesCollector());

@jpavel
Copy link
Contributor Author

jpavel commented Dec 9, 2013

Sure, sorry for that - it is unremoved artifact of testing

@thspeer
Copy link
Contributor

thspeer commented Dec 9, 2013

You will need to remove all these, but this means that all helper classes need to have
edm::ConsumesCollector && iC -> with 2 &, not only 1
if a helper needs another helper and has to pass this iC on, use std::move(iC)

@Dr15Jones
Copy link
Contributor

@thspeer You shouldn't use 'chains' of moves. E.g.

foo(std::move(bar));
fie(std::move(bar));

The call to fie will get an empty Bar since the contents were moved during the call to foo.

@jpavel
Copy link
Contributor Author

jpavel commented Dec 9, 2013

Ok - so what is recommended approach? We have one helper that is accessed by many helpers that are called by tau and piZero produceres. I have seen in the other packages that people create an instance of consumesCollector from consumesCollector() and pass this one.

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 9, 2013

@cmsbuild
Copy link
Contributor

Pull request #1718 was updated. @nclopezo, @cmsbuild, @thspeer, @slava77 can you please check and sign again.

@cmsbuild
Copy link
Contributor

-1
When I ran the RelVals I found an error in the following worklfows:
1306.0 step1

runTheMatrix-results/1306.0_SingleMuPt1_UP15+SingleMuPt1_UP15+DIGIUP15+RECOUP15+HARVESTUP15/step1_SingleMuPt1_UP15+SingleMuPt1_UP15+DIGIUP15+RECOUP15+HARVESTUP15.log
----- Begin Fatal Exception 11-Dec-2013 09:41:36 CET-----------------------
An exception of category 'StdException' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing ESSource: class=PoolDBESSource label='GlobalTag'
Exception Message:
A std::exception was thrown.
Can not get data (Additional Information: [frontier.c:1034]: No more proxies. Last error was: Request 60 on chan 2 failed at Wed Dec 11 09:41:36 2013: -8 [payload.c:105]: Server signalled payload error 1: FrontierPrep java.sql.SQLException: Listener refused the connection with the following error: ORA-12514, TNS:listener does not currently know of service requested in connect descriptor at oracle.jdbc.driver.T4CConnection.logon(T4CConnection.java:512)) ( CORAL : "coral::FrontierAccess::Statement::execute" from "CORAL/RelationalPlugins/frontier" )
----- End Fatal Exception -------------------------------------------------

4.53 step2

runTheMatrix-results/4.53_RunPhoton2012B+RunPhoton2012B+HLTD+RECODreHLT+HARVESTDreHLT/step2_RunPhoton2012B+RunPhoton2012B+HLTD+RECODreHLT+HARVESTDreHLT.log
----- Begin Fatal Exception 11-Dec-2013 09:41:39 CET-----------------------
An exception of category 'StdException' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing ESSource: class=PoolDBESSource label='GlobalTag'
Exception Message:
A std::exception was thrown.
Can not get data (Additional Information: [frontier.c:1034]: No more proxies. Last error was: Request 60 on chan 2 failed at Wed Dec 11 09:41:39 2013: -8 [payload.c:105]: Server signalled payload error 1: FrontierPrep java.sql.SQLException: Listener refused the connection with the following error: ORA-12514, TNS:listener does not currently know of service requested in connect descriptor at oracle.jdbc.driver.T4CConnection.logon(T4CConnection.java:512)) ( CORAL : "coral::FrontierAccess::Statement::execute" from "CORAL/RelationalPlugins/frontier" )
----- End Fatal Exception -------------------------------------------------

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

@jpavel
Copy link
Contributor Author

jpavel commented Dec 11, 2013

I think this error is not caused by PFTau sequence but by unavailability of the input data, right?

@ktf
Copy link
Contributor

ktf commented Dec 11, 2013

Looks like a frontier glitch, indeed. @nclopezo can you resubmit the tests?

Sent from my iPhone

On 11/dic/2013, at 10:02, Pavel Jez notifications@github.com wrote:

I think this error is not caused by PFTau sequence but by unavailability of the input data, right?


Reply to this email directly or view it on GitHub.

@thspeer
Copy link
Contributor

thspeer commented Dec 11, 2013

Working @thspeer

@cmsbuild
Copy link
Contributor

@thspeer
Copy link
Contributor

thspeer commented Dec 11, 2013

+1
Tested d4417d5 in CMSSW_7_0_X_2013-12-11-0200-1718
No difference in reco as expected, based on RelMon and reco script

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next IBs unless changes or unless it breaks tests. @ktf can you please take care of it?

ktf added a commit that referenced this pull request Dec 12, 2013
…onsumes

Reco update -- Added consumes interface to RecoTau sequence
@ktf ktf merged commit 23de6c7 into cms-sw:CMSSW_7_0_X Dec 12, 2013
@jpavel jpavel deleted the CMSSW_7_0_X_2013-12-07-0200_TAUconsumes branch January 21, 2014 11:03
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

5 participants