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

RecoLocalCalo/EcalRecProducers and RecoEcal/EgammaClusterProducer : consumes migration #663

Closed
wants to merge 6 commits into from

Conversation

argiro
Copy link
Contributor

@argiro argiro commented Aug 29, 2013

with additional cleanups and removal of dead wood
FWCore/Utilities/interface/transform.h is from pull request #648

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @argiro for CMSSW_7_0_X.

RecoLocalCalo/EcalRecProducers and RecoEcal/EgammaClusterProducer : consumes migration

It involves the following packages:

FWCore/Utilities
RecoLocalCalo/EcalRecProducers
RecoEcal/EgammaClusterProducers

@Dr15Jones, @ktf, @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.

@Dr15Jones
Copy link
Contributor

-1
This should be rebased on top of #648 rather than having that pulls change as part of a larger commit in this pull request.

@argiro
Copy link
Contributor Author

argiro commented Aug 29, 2013

On 29 Aug 2013, at 18:12, Chris Jones notifications@github.com wrote:

-1
This should be rebased on top of #648 rather than having that pulls change as part of a larger commit in this pull request.

can it be done by the integrator (should be easy for him) or do I have to take it off my branch ?


Reply to this email directly or view it on GitHub.

@argiro
Copy link
Contributor Author

argiro commented Aug 30, 2013

Hi,

I removed FWCore/Utilities/interface/transform.h from this branch

S.

On 29 Aug 2013, at 23:07, Stefano Argiro' stefano.argiro@cern.ch wrote:

On 29 Aug 2013, at 18:12, Chris Jones notifications@github.com wrote:

-1
This should be rebased on top of #648 rather than having that pulls change as part of a larger commit in this pull request.

can it be done by the integrator (should be easy for him) or do I have to take it off my branch ?


Reply to this email directly or view it on GitHub.

@thspeer
Copy link
Contributor

thspeer commented Aug 30, 2013

Working @thspeer

@cmsbuild
Copy link
Contributor

Pull request #663 was updated. Signatures reset, please check and sign again.

@thspeer
Copy link
Contributor

thspeer commented Sep 2, 2013

Could you please delete RecoLocalCalo/EcalRecProducers/data_RAW2DIGI_L1Reco_RECO.root

hitproducer_ = ps.getParameter<std::string>("ecalhitproducer");
hitcollection_ =ps.getParameter<std::string>("ecalhitcollection");
hitsToken_ =
consumes<EcalRecHitCollection>(ps.getParameter<edm::InputTag>("recHitsCollection"));
Copy link
Contributor

Choose a reason for hiding this comment

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

You have changed the input from two strings to an input tag, but this change is not propagated the change to the HLT. Please keep the same parameters, or coordinate this with the HLT. I get the error:

[1] Constructing module: class=HybridClusterProducer label='hltHybridSuperClustersActivity'
Exception Message:
MissingParameter: Parameter 'recHitsCollection' not found.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi [ add Matteo cc],

I think the idea is to change HLT code too

cheers

Stefano

On 2 Sep 2013, at 16:43, thspeer notifications@github.com wrote:

In RecoEcal/EgammaClusterProducers/src/HybridClusterProducer.cc:

@@ -43,8 +42,8 @@

basicclusterCollection_ = ps.getParameterstd::string("basicclusterCollection");
superclusterCollection_ = ps.getParameterstd::string("superclusterCollection");

  • hitproducer_ = ps.getParameterstd::string("ecalhitproducer");
  • hitcollection_ =ps.getParameterstd::string("ecalhitcollection");
  • hitsToken_ =
  • consumes(ps.getParameteredm::InputTag("recHitsCollection"));

You have changed the input from two strings to an input tag, but this change is not propagated the change to the HLT. Please keep the same parameters, or coordinate this with the HLT. I get the error:

[1] Constructing module: class=HybridClusterProducer label='hltHybridSuperClustersActivity'
Exception Message:
MissingParameter: Parameter 'recHitsCollection' not found.


Reply to this email directly or view it on GitHub.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I have changed it too.
If you want I can make a pull request with the update.

On 09/02/2013 04:49 PM, Stefano Argiro' wrote:

Hi [ add Matteo cc],

I think the idea is to change HLT code too

cheers

Stefano

On 2 Sep 2013, at 16:43, thspeer notifications@github.com wrote:

In RecoEcal/EgammaClusterProducers/src/HybridClusterProducer.cc:

@@ -43,8 +42,8 @@

basicclusterCollection_ = ps.getParameter<std::string>("basicclusterCollection");
superclusterCollection_ = ps.getParameter<std::string>("superclusterCollection");
  • hitproducer_ = ps.getParameterstd::string("ecalhitproducer");
  • hitcollection_ =ps.getParameterstd::string("ecalhitcollection");
  • hitsToken_ =
  • consumes(ps.getParameteredm::InputTag("recHitsCollection"));
    You have changed the input from two strings to an input tag, but this change is not propagated the change to the HLT. Please keep the same parameters, or coordinate this with the HLT. I get the error:

[1] Constructing module: class=HybridClusterProducer label='hltHybridSuperClustersActivity'
Exception Message:
MissingParameter: Parameter 'recHitsCollection' not found.


Reply to this email directly or view it on GitHub.

Ciao,
Matteo

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 2, 2013

Pull request #663 was updated. Signatures reset, please check and sign again.

@argiro
Copy link
Contributor Author

argiro commented Sep 2, 2013

On 2 Sep 2013, at 16:30, thspeer notifications@github.com wrote:

Could you please delete RecoLocalCalo/EcalRecProducers/data_RAW2DIGI_L1Reco_RECO.root

done, sorry about that

S.


Reply to this email directly or view it on GitHub.

@ktf
Copy link
Contributor

ktf commented Sep 10, 2013

Stefano your branch does not merge with CMSSW_7_0_X. Can you rebase it on top of it? Looks like some files you removed got modified...

@argiro
Copy link
Contributor Author

argiro commented Sep 11, 2013

I have re-based, with some additional changes, and issued a new pull request #778. We can close this one

S.

On 10 Sep 2013, at 12:01, Giulio Eulisse notifications@github.com wrote:

Stefano your branch does not merge with CMSSW_7_0_X. Can you rebase it on top of it? Looks like some files you removed got modified...


Reply to this email directly or view it on GitHub.

@ktf
Copy link
Contributor

ktf commented Sep 11, 2013

You should be able to close it yourself. Is this not the case?

@ktf ktf closed this Sep 11, 2013
@argiro
Copy link
Contributor Author

argiro commented Sep 11, 2013

On 11 Sep 2013, at 11:17, Giulio Eulisse notifications@github.com wrote:

You should be able to close it yourself. Is this not the case?

yes, but it seems you've been quicker…
S.


Reply to this email directly or view it on GitHub.

@ktf
Copy link
Contributor

ktf commented Sep 11, 2013

Ok. Just checking this was the case.

clelange pushed a commit to clelange/cmssw that referenced this pull request Nov 17, 2016
arizzi added a commit to arizzi/cmssw that referenced this pull request Feb 13, 2017
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