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

remove phase2_hgcalV9 modifier #30702

Merged
merged 1 commit into from
Jul 21, 2020
Merged

remove phase2_hgcalV9 modifier #30702

merged 1 commit into from
Jul 21, 2020

Conversation

kpedro88
Copy link
Contributor

PR description:

After #30645, the phase2_hgcalV9 modifier and various features to support the old HGCal geometry (which used HCAL for the scintillator part) are no longer needed.

This PR removes the modifier and simplifies the associated configuration files. In some cases, the former modifications were set as the default or as part of the base phase2_hgcal modifier; in other cases (e.g. tuning/calibrations), the modifications were moved into the phase2_hgcalV10 modifier.

If HGCal experts (@cseez @rovere @bsunanda @jbsauvan) would prefer different choices in some of these cases, please comment on this PR.

This PR only addresses configuration files (and associated fillDescriptions()). Significant parts of the C++ code could be cleaned up further, but this is left to the existing issue #27095.

PR validation:

Ran workflow 23234.0 successfully.

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-30702/17005

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @kpedro88 (Kevin Pedro) for master.

It involves the following packages:

Configuration/Eras
Configuration/Geometry
Configuration/StandardSequences
Geometry/HGCalGeometry
L1Trigger/L1THGCal
L1Trigger/L1THGCalUtilities
RecoLocalCalo/HGCalRecProducers
RecoParticleFlow/PFClusterProducer
SimCalorimetry/HGCalSimProducers
SimGeneral/MixingModule
Validation/HGCalValidation

@andrius-k, @schneiml, @ianna, @kpedro88, @rekovic, @fioriNTU, @perrotta, @civanch, @silviodonato, @cmsbuild, @davidlange6, @Dr15Jones, @cvuosalo, @mdhildreth, @jfernan2, @slava77, @jpata, @fabiocos, @benkrikler, @kmaeshima, @makortel, @franzoni can you please review it and eventually sign? Thanks.
@felicepantaleo, @Martin-Grunewald, @bsunanda, @jbsauvan, @lgray, @seemasharmafnal, @mmarionncern, @sethzenz, @vargasa, @makortel, @dgulhan, @apsallid, @slomeo, @vandreev11, @GiacomoSguazzoni, @rovere, @VinInn, @cseez, @deguio, @hatakeyamak, @mtosi, @fabiocos, @clelange, @riga, @edjtscott, @lecriste, @cbernet, @amarini, @pfs this is something you requested to watch as well.
@silviodonato, @dpiparo you are the release manager for this.

cms-bot commands are listed here

@kpedro88
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 15, 2020

The tests are being triggered in jenkins.

@bsunanda
Copy link
Contributor

@kpedro88 I have a few corrections for dict2026. I shall wait till this PR is integrated. Meanwhile I have corrected the local cfi's for HGCal

@cmsbuild
Copy link
Contributor

+1
Tested at: ec631a6
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c42357/7968/summary.html
CMSSW: CMSSW_11_2_X_2020-07-14-2300
SCRAM_ARCH: slc7_amd64_gcc820

@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-c42357/7968/summary.html

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

  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-c42357/28234.0_TTbar_14TeV+TTbar_14TeV_TuneCP5_2026D60_GenSimHLBeamSpotFull14+DigiFullTrigger_2026D60+RecoFullGlobal_2026D60+HARVESTFullGlobal_2026D60

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 35
  • DQMHistoTests: Total histograms compared: 2612943
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2612894
  • DQMHistoTests: Total skipped: 48
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 34 files compared)
  • Checked 146 log files, 17 edm output root files, 35 DQM output files

@kpedro88
Copy link
Contributor Author

+upgrade

@jfernan2
Copy link
Contributor

+1

@cvuosalo
Copy link
Contributor

+1

@jbsauvan
Copy link
Contributor

Thanks @kpedro88, the L1THGCal changes look fine for me.

@slava77
Copy link
Contributor

slava77 commented Jul 16, 2020

+1

for #30702 ec631a6

  • code changes are in line with the PR description
  • jenkins tests pass and comparisons with the baseline show no differences (apparently because pre-v9 tests were removed already)

@civanch
Copy link
Contributor

civanch commented Jul 17, 2020

+1

@slava77
Copy link
Contributor

slava77 commented Jul 20, 2020

l1 signature is missing
please check/sign

@silviodonato
Copy link
Contributor

+operations

@silviodonato
Copy link
Contributor

@rekovic ?

@silviodonato
Copy link
Contributor

merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment