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
data processing setup for 2017 era #17635
data processing setup for 2017 era #17635
Conversation
… not in the process [fragment]
A new Pull Request was created by @slava77 (Slava Krutelyov) for CMSSW_9_0_X. It involves the following packages: Configuration/DataProcessing @perrotta, @cmsbuild, @silviodonato, @Martin-Grunewald, @franzoni, @fwyzard, @davidlange6 can you please review it and eventually sign? Thanks. cms-bot commands are listed here #13028 |
@cmsbuild please test |
The tests are being triggered in jenkins. |
Comparison job queued. |
@@ -27,6 +27,9 @@ def modifyHLTPhaseIPixelGeom(process): | |||
|
|||
# modify the HLT configuration to run the Phase I tracking in the particle flow sequence | |||
def customizeHLTForPFTrackingPhaseI2017(process): | |||
if not hasattr(process, 'hltPixelLayerTriplets'): | |||
#there could also be a message here that the call is done for non-HLT stuff | |||
return process; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it because some HLT configurations do not have hltPixelLayerTriplets
& co., or because this ends up being callen for non-HLT configurations ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the latter, this file is loaded (indirectly) by Configuration.Skimming.Skims_PDWG_cff in non-HLT configuration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see - but the Era-based customisation should still affect only the HLT part, right ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, but it can't customise something that doesn't already exist in the HLT part. That's why this "if hasattr" is here
+1 |
from Configuration.Eras.Era_Run2_2017_cff import Run2_2017 | ||
from Configuration.Eras.Modifier_trackingLowPU_cff import trackingLowPU | ||
|
||
Run2_2017_trackingLowPU = cms.ModifierChain(Run2_2017, trackingLowPU) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here Run2_2017_core
should be used instead of Run2_2017
. Adding trackingLowPU
will likely have strange effects as trackingPhase1QuadProp
is already active, and several automated constructs assume only one of the tracking sub-eras being active (hmm, maybe I should try to think how to add a check for that...). OTOH, run2_GEM_2017
is currently only in Run2_2017
while IMHO it should be in Run2_2017_core
. Further OTOH, after #17605 and with #17612 we should anyway simplify the era hierarchy (I guess it's mostly up to deciding to who does the job).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can do the changes, but it sounds like it's more than just changing to Run2_2017_core
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doing all the cleanup/restructuring, yes, that would be more than switching to run2_2017_core
. I can also do the cleanup part if you prefer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if the cleanup was made already, will I still need to change this to run2_2017_core or will this part change as well?
If there is coupling, I'd rather leave this as is.
Matti, please clarify.
Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@slava77 It will need a change, which will conflict with changing it to run2_2017_core
. Because of the coupling, I agree it's easiest to leave as it is (since it wll be fixed soon).
Other thought: if trackingLowPU
configuration should be kept working also in 2017, it would probably be good to add a matrix workflow for it, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, a workflow with it would be good to have (not in the short matrix used by jenkins though)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kpedro88 and @abdoulline may want to follow this as well In order to test meaningfully ppEra_Run2_2017 we've created a candidate global tag 90X_dataRun2_Prompt_Candidate_2017_02_27_07_32_56_ONLY_FOR_TESTS including, on top of what was used for the replay with 2016-like conditions ( diff with respect to 90X_dataRun2_Prompt_v1 ) : . the GEM geometry Testing such GT on a recent run from MWGR1 using CMSSW_9_0_X_2017-02-25-1100 + this PR:
we get an error |
we also have a candiate GT from the HTL queue: 90X_dataRun2_HLT_Candidate_2017_02_27_07_32_58_ONLY_FOR_TESTS @degrutto, please take note of it. We have not done equivalent tests to what done for prompt. |
@franzoni @davidlange6 (operations signatories) please check and sign or suggest changes if needed. |
was just waiting for the discussion to conclude. |
DISCARD THIS MESSAGE @franzoni
|
we are anyway going to investigate what the source of the problem is |
@deguio |
you are right, I realized that. wrong file. |
let me report here as well. I've tried with a global run from yesterday that @abenagli provided: here is the file: here is the command: few events, but no crashes and clean logs (at least from hcal side). |
this is understood. reporting below the explanation by @abenagli. the unpacker is fine, as well as the emap. If run 287961 was used for these checks, a pure ngHE F/W was loaded into crate 11, so I’m guessing that all channels of that µHTR have been flagged as “ng” by the µHTR, including the HB ones, A first working version of the mixed uHTR F/W has been deployed in crate 11 on Feb. 24th, so only runs after that date should be used for reconstruction tests. In particular, Sun > Mon overnight run is a good candidate (288236). ~deguio/public/ForKevin/USC_288236_streamDQM.root |
Thanks @deguio for the explanations and additional tests |
Added scenarios:
Additional changes to make dataProcessing to work:
Tested with an "uber-config"
2016 era:
Similar config for 2017 parses OK but doesn't run for data due to GT inconsistencies.
Similar config for 2017 with MC runs OK (with HcalCalIterativePhiSym removed)