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

HCAL 2017 Phase1 Workflows #15889

Merged
merged 18 commits into from
Sep 23, 2016
+454 −12
Merged

HCAL 2017 Phase1 Workflows #15889

Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
a4e6c05
add additional hcal 2017 sub-Eras
kpedro88 Sep 8, 2016
bf5f1f2
customizations for hcal phase1 packer
kpedro88 Sep 9, 2016
4eb39a0
customizations for hcal phase1 reco
kpedro88 Sep 9, 2016
0afa935
customizations for hcal phase1 dqm
kpedro88 Sep 9, 2016
bfe444b
fix hf prereco collection
kpedro88 Sep 12, 2016
ff46989
update geometry for 2017 workflow
kpedro88 Sep 13, 2016
61abaf5
use hcaldev tag for workflow
kpedro88 Sep 17, 2016
e3418e2
safer sequence inserts
kpedro88 Sep 18, 2016
909daed
use HFFlexibleTimeCheck
kpedro88 Sep 19, 2016
1046942
add usage comments for HCAL 2017 Eras
kpedro88 Sep 19, 2016
256242e
separate Era and workflow for HCALdev 2017
kpedro88 Sep 21, 2016
8393d1a
use DB geom for HCAL 2017
kpedro88 Sep 21, 2016
68b20fd
add Era_Run2_2017_HCALdev
kpedro88 Sep 21, 2016
7fadf6c
properly sync 2017 eras
kpedro88 Sep 22, 2016
b1543bb
revert to xml geom
kpedro88 Sep 22, 2016
a14deaa
add combined '2017 new' workflow with all HCAL and pixel changes
kpedro88 Sep 22, 2016
833fe0e
add 2017 new WF to short matrix
kpedro88 Sep 22, 2016
21e6fec
turn off HBHEChannelInfo
kpedro88 Sep 22, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
use hcaldev tag for workflow
  • Loading branch information
@kpedro88
kpedro88 committed Sep 22, 2016
commit 61abaf563b395e66c84a3942eebed8cc616c1735
2 changes: 1 addition & 1 deletion Configuration/PyReleaseValidation/python/upgradeWorkflowComponents.py
View file
Original file line number Diff line number Diff line change
@@ -57,7 +57,7 @@
upgradeProperties[2017] = {
'2017' : {
'Geom' : 'Extended2017dev',
Copy link
Contributor

Choose a reason for hiding this comment

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

@kpedro88 - no need to change it from DB:Extended if taken with #15924

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll change it back when #15924 is merged

'GT' : 'auto:phase1_2017_realistic',
'GT' : 'auto:phase1_2017_hcaldev',
Copy link
Contributor

Choose a reason for hiding this comment

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

I presume this will switch pixel geometry (well, everything) to be read from XML. In which pre-release is this PR planned to be integrated? (pre12, pre13; or will the geometry be included in a GT before the pre-release?)

I'm wondering a possible interference with #15883 and #15805 (or actually with their validation).

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, this sounds like it could cause great chaos if it gets into pre12. And for pre13/the release, I have the feeling that nothing called 'dev' should be in the then-production workflows.

It seems to me that Extended2017dev should be merged into the standard 2017 geometry for the release, and ideally after pre12 to make the tracker validation not more complicated than it is already. Does this dev geometry still need validation? It seems last significant changes were in May.

Copy link
Contributor

Choose a reason for hiding this comment

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

Note that #15674 changed Extended2017 but not the dev version, so merging might be nontrivial.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The 2017dev geometry itself does not need validation, but it can only be used (for DIGI-RECO) with an appropriate global tag having HCAL conditions for all the new channels. As we just got one, it could not have been used in the workflow until now. @ianna doesn't have the latest version of it in the database yet, so I assigned the xml version for now.

This shouldn't "interfere" with the new pixel geometry PRs, as those are creating a separate workflow for validation. @davidlange6 requested that I not make yet another separate workflow for HCAL 2017.

At some point, all of these geometry changes need to be reconciled into one final 2017 geometry, but I think that should be done after or otherwise outside of this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

I had the impression that the goal of pre13 was to "merge" the "2017_NewFPix" workflow introduced in #15805 back to the "2017" workflow. Personally I don't have a problem in keeping it around if needed for other reasons, just all cleanup needs to be done in there (although maybe not much can be done before the "2017" and "2017_NewFPix" get really merged?).

For my education, are the additions in Run2_2017 era deeply coupled with this geometry? I mean, does the Run2_2017 work/make any sense with the DB:Extended geometry?

FYI @rovere @VinInn @boudoul @veszpv

Copy link
Contributor

Choose a reason for hiding this comment

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

This shouldn't "interfere" with the new pixel geometry PRs, as those are creating a separate workflow for validation.

This is not strictly correct, the new WFs exist to evaluate the new FPIX individually. There are aso BPIX changes that need validation in the normal 2017 geometry. However, these are not as drastic as the FPIX changes and might be fine to coexist with the HCAL changes.

Copy link
Contributor

Choose a reason for hiding this comment

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

There are aso BPIX changes that need validation in the normal 2017 geometry.

It is not just the BPix changes we need to validate with pre12, but also all non-geometry changes between pre11 and pre12 (e.g. turning off VPF30 mitigation), and that is best done with exactly the same geometry as in pre11.

Now, trying to be practical (probably along what @slava77 already wrote), as these two parameters are passed to cmsDriver they can be changed at the time of RelVal submission (to be coordinated with PdmV; by the way, if this PR goes in pre12, do you want RelVals with the Extended2017dev geometry?).

So my only concern is the effect of the run2_HF_2017 and run2_HCAL_2017 sub-eras when run on 2016 HCAL geometry (both when re-using pre11 GEN-SIM and generating new). If the effect is statistically insignificant (within the usual 9k events) on high-pT jets (i.e. jetCore tracking iteration and pixel cluster splitting), then I'm personally fine with changing the "2017" workflow geometry to Extended2017dev for pre12.

Copy link
Contributor

Choose a reason for hiding this comment

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

nominally, undoing a change implemented with an era, should be another era,
but we are told to not use it and now discuss how to turn things off instead with a cmsDriver command.

If I understand correctly, changes made by HCAL eras in this PR will not work on the old 2016 HCAL geometry.

Copy link
Contributor

Choose a reason for hiding this comment

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

@slava77 - yes, HCal and CaloTower reco geometries produced from a 2017dev scenario differ from the ones produced from a 2017 scenario.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@makortel the changes from the new HCAL geometry, tag, and customizations definitely are not statistically insignificant. You cannot use the new HCAL tag with pre11 GEN-SIM.

I first suggested making a separate testing workflow and Era for the HCAL changes, but @davidlange6 did not want me to do this. At this point, he should comment if he would accept this PR with such a (temporary) new workflow and Era before I make those changes.

'Era' : 'Run2_2017',
'ScenToRun' : ['GenSimFull','DigiFull','RecoFull','HARVESTFull'],
},