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

Feature Bits for 2017 HCAL HF TP #16156

Merged
merged 11 commits into from Oct 18, 2016

Conversation

matz-e
Copy link
Contributor

@matz-e matz-e commented Oct 9, 2016

Sits on top of #15945.

  • Refactors the channel validity check to happen earlier
  • Port of the 2016 MinBias feature bit
  • Refactor the EM feature bit and hook it up to 2017 TP (if manually configured in the PSet)

Allows for finer-grain overwriting, and still taking some parameters from
the database.
Simplify code; Don't de- and then re-construct a dataframe, just pass it
through.  Unify the energy reconstruction in preparation for dual-anode
readout.
@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 9, 2016

A new Pull Request was created by @matz-e (Matthias Wolf) for CMSSW_8_1_X.

It involves the following packages:

SimCalorimetry/HcalTrigPrimAlgos
SimCalorimetry/HcalTrigPrimProducers

@cmsbuild, @rekovic, @mulhearn, @davidlange6 can you please review it and eventually sign? Thanks.
@slava77, @smuzaffar you are the release manager for this.

cms-bot commands are list here #13028

@kpedro88
Copy link
Contributor

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 10, 2016

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/15625/console

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@matz-e
Copy link
Contributor Author

matz-e commented Oct 11, 2016

Again, @cmsbuild says comparisons are ready, but the linked page shows them as "Running"…

@kpedro88
Copy link
Contributor

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 12, 2016

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/15684/console

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_8_1_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @smuzaffar

auto adc_threshold = parameters->getADCThresholdHF();
auto tdc_mask = parameters->getTDCMaskHF();

if (override_parameters_.exists("ADCThresholdHF"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @matz-e - we can't do this sort of check for every digi. can you see who to move it to a once per job check. Thanks

// finegrain[ibin] = (finegrain[ibin] || hcalfem->fineGrainbit(ADCShort, details.ShortDigi.id(), details.ShortDigi[ibin].capid(), ADCLong, details.LongDigi.id(), details.LongDigi[ibin].capid()));
// }
for (const auto& detail: details) {
if (detail.validity[idx] and HcalDetId(detail.digi.id()).ietaAbs() != 29) {
Copy link
Contributor

Choose a reason for hiding this comment

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

please document the magic number 29

@cmsbuild
Copy link
Contributor

Pull request #16156 was updated. @cmsbuild, @rekovic, @mulhearn, @davidlange6 can you please check and sign again.

@kpedro88
Copy link
Contributor

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 17, 2016

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/15778/console

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

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

@slava77 comparisons for the following workflows were not done due to missing matrix map:

  • 20024.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2023D1_GenSimHLBeamSpotFull+DigiFull_2023D1+RecoFullGlobal_2023D1+HARVESTFullGlobal_2023D1
  • 22424.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2023D3Timing_GenSimHLBeamSpotFull+DigiFull_2023D3Timing+RecoFullGlobal_2023D3Timing+HARVESTFullGlobal_2023D3Timing

@matz-e
Copy link
Contributor Author

matz-e commented Oct 18, 2016

Ping @mulhearn, @rekovic for re-approval?

@davidlange6 davidlange6 merged commit 46c7c4d into cms-sw:CMSSW_8_1_X Oct 18, 2016
@matz-e matz-e deleted the hcal-tp-2017-db-v2 branch September 15, 2017 15:01
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