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.

Sign up for GitHub

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

Jump to bottom

implement "plan 1" and sum SiPM rechits over all depths #17646

Merged
merged 1 commit into from Mar 2, 2017
Merged

implement "plan 1" and sum SiPM rechits over all depths #17646

merged 1 commit into from Mar 2, 2017

Conversation

fwyzard
Copy link
Contributor

@fwyzard fwyzard commented Feb 27, 2017

Introduce a new name hltHbhePhase1Reco for the collection of rechits with QIE8/QIE11 and depth segmentation while keeping the original name hltHbhereco for the collection of rechits summed over all depths.

@fwyzard
implement "plan 1" and sum SiPM rechits over all depths
f80d8c9 
introduce a new name (hltHbhePhase1Reco) for the collection of rechits with
QIE8/QIE11 and depth segmentation while keeping the original name (hltHbhereco)
for the collection of rechits summed over all depths
@fwyzard
Copy link
Contributor Author

fwyzard commented Feb 27, 2017

please test

@cmsbuild cmsbuild added this to the Next CMSSW_9_0_X milestone Feb 27, 2017
@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 27, 2017 •
edited

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/17995/console Started: 2017/02/27 20:20

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @fwyzard (Andrea Bocci) for CMSSW_9_0_X.

It involves the following packages:

HLTrigger/Configuration

@perrotta, @cmsbuild, @silviodonato, @Martin-Grunewald, @fwyzard, @davidlange6 can you please review it and eventually sign? Thanks.
@makortel, @Martin-Grunewald, @geoff-smith, @jalimena this is something you requested to watch as well.
@davidlange6, @smuzaffar you are the release manager for this.

cms-bot commands are listed here #13028

@fwyzard
Copy link
Contributor Author

fwyzard commented Feb 27, 2017

+1

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_9_0_X IBs after it passes the integration tests. This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @smuzaffar

@fwyzard
Copy link
Contributor Author

fwyzard commented Feb 27, 2017

I guess in principle these changes should come from the run2_HEPlan1_2017 era, but that would be a bit more complicated to support.
Do let me know if having them in the common run2_HCAL_2017 era breaks anything.

@cmsbuild
Copy link
Contributor

Pull request #17646 was updated. @perrotta, @cmsbuild, @silviodonato, @Martin-Grunewald, @fwyzard, @davidlange6 can you please check and sign again.

@fwyzard
Copy link
Contributor Author

fwyzard commented Feb 27, 2017

please test

@fwyzard
Copy link
Contributor Author

fwyzard commented Feb 27, 2017

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 27, 2017 •
edited

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/17996/console Started: 2017/02/27 20:38

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_9_0_X IBs after it passes the integration tests. This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @smuzaffar

@fwyzard
Copy link
Contributor Author

fwyzard commented Feb 27, 2017

sorry, pushed to the wrong branch

@fwyzard
Copy link
Contributor Author

fwyzard commented Feb 27, 2017

+1

@fwyzard
Copy link
Contributor Author

fwyzard commented Feb 27, 2017

please test

@fwyzard
Copy link
Contributor Author

fwyzard commented Feb 27, 2017

(not sure if the tests are needed again or not)

@cmsbuild
Copy link
Contributor

+1
Tested at: f80d8c9
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-17646/17996/summary.html

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-17646/17996/summary.html

@Martin-Grunewald
Copy link
Contributor

@fwyzard
Does this way avoid expansion of sequences when adding/inserting?

@fwyzard
Copy link
Contributor Author

fwyzard commented Feb 28, 2017

Yes, it should avoid it.

@Martin-Grunewald
Copy link
Contributor

Great!

@davidlange6
Copy link
Contributor

+1
(but depending on names of plugins is dangerous at best)

@cmsbuild cmsbuild merged commit 8734292 into cms-sw:CMSSW_9_0_X Mar 2, 2017
@fwyzard
Copy link
Contributor Author

fwyzard commented Mar 2, 2017

Thanks.

What do you mean by

depending on names of plugins

?

@cmsbuild cmsbuild mentioned this pull request Mar 2, 2017
Merged
@kpedro88
Copy link
Contributor

kpedro88 commented Mar 2, 2017

@fwyzard just saw this - you can get away with keeping these changes in Era run2_HCAL_2017 as long as we keep Plan1; if it gets removed, you should revert

@kpedro88
Copy link
Contributor

kpedro88 commented Mar 2, 2017

does this also need to be ported to 84X?

@fwyzard
Copy link
Contributor Author

fwyzard commented Mar 2, 2017

Thanks, let's keep it like that then.

No need to backport to 8.4.x if it is used only for data taking. We should backport it if it is used for MC production.

@davidlange6 davidlange6 mentioned this pull request Mar 24, 2017
Closed
@fwyzard fwyzard deleted the update_HCAL_customisation branch July 11, 2017 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
comparison-available fully-signed hlt-approved orp-approved tests-approved
Projects
None yet
Milestone
CMSSW_9_0_X
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@fwyzard @cmsbuild @Martin-Grunewald @davidlange6 @kpedro88