-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
MF regression IB tests #31651
MF regression IB tests #31651
Conversation
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-31651/18750
|
A new Pull Request was created by @namapane (Nicola Amapane) for master. It involves the following packages: Configuration/StandardSequences @perrotta, @silviodonato, @cmsbuild, @franzoni, @slava77, @jpata, @qliphy, @fabiocos, @davidlange6 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
@cmsbuild please test |
The tests are being triggered in jenkins.
|
-1 Tested at: fe8e526 CMSSW: CMSSW_11_2_X_2020-10-02-2300 I found follow errors while testing this PR Failed tests: RelVals AddOn
When I ran the RelVals I found an error in the following workflows: runTheMatrix-results/4.22_RunCosmics2011A+RunCosmics2011A+RECOCOSD+ALCACOSD+SKIMCOSD+HARVESTDC/step2_RunCosmics2011A+RunCosmics2011A+RECOCOSD+ALCACOSD+SKIMCOSD+HARVESTDC.log4.53 step2 runTheMatrix-results/4.53_RunPhoton2012B+RunPhoton2012B+HLTD+RECODR1reHLT+HARVESTDR1reHLT/step2_RunPhoton2012B+RunPhoton2012B+HLTD+RECODR1reHLT+HARVESTDR1reHLT.log136.731 step2 runTheMatrix-results/136.731_RunSinglePh2016B+RunSinglePh2016B+HLTDR2_2016+RECODR2_2016reHLT_skimSinglePh_HIPM+HARVESTDR2/step2_RunSinglePh2016B+RunSinglePh2016B+HLTDR2_2016+RECODR2_2016reHLT_skimSinglePh_HIPM+HARVESTDR2.log136.7611 step2 runTheMatrix-results/136.7611_RunJetHT2016E_reminiaod+RunJetHT2016E_reminiaod+REMINIAOD_data2016_HIPM+HARVESTDR2_REMINIAOD_data2016_HIPM/step2_RunJetHT2016E_reminiaod+RunJetHT2016E_reminiaod+REMINIAOD_data2016_HIPM+HARVESTDR2_REMINIAOD_data2016_HIPM.log136.793 step2 runTheMatrix-results/136.793_RunDoubleEG2017C+RunDoubleEG2017C+HLTDR2_2017+RECODR2_2017reHLT_skimDoubleEG_Prompt+HARVEST2017/step2_RunDoubleEG2017C+RunDoubleEG2017C+HLTDR2_2017+RECODR2_2017reHLT_skimDoubleEG_Prompt+HARVEST2017.log136.8311 step2 runTheMatrix-results/136.8311_RunJetHT2017F_reminiaod+RunJetHT2017F_reminiaod+REMINIAOD_data2017+HARVEST2017_REMINIAOD_data2017/step2_RunJetHT2017F_reminiaod+RunJetHT2017F_reminiaod+REMINIAOD_data2017+HARVEST2017_REMINIAOD_data2017.log136.874 step2 runTheMatrix-results/136.874_RunEGamma2018C+RunEGamma2018C+HLTDR2_2018+RECODR2_2018reHLT_skimEGamma_Offline_L1TEgDQM+HARVEST2018_L1TEgDQM/step2_RunEGamma2018C+RunEGamma2018C+HLTDR2_2018+RECODR2_2018reHLT_skimEGamma_Offline_L1TEgDQM+HARVEST2018_L1TEgDQM.log136.88811 step2 runTheMatrix-results/136.88811_RunJetHT2018D_reminiaodUL+RunJetHT2018D_reminiaodUL+REMINIAOD_data2018UL+HARVEST2018_REMINIAOD_data2018UL/step2_RunJetHT2018D_reminiaodUL+RunJetHT2018D_reminiaodUL+REMINIAOD_data2018UL+HARVEST2018_REMINIAOD_data2018UL.log140.56 step2 runTheMatrix-results/140.56_RunHI2018+RunHI2018+RECOHID18+HARVESTDHI18/step2_RunHI2018+RunHI2018+RECOHID18+HARVESTDHI18.log140.53 step2 runTheMatrix-results/140.53_RunHI2011+RunHI2011+RECOHID11+HARVESTDHI/step2_RunHI2011+RunHI2011+RECOHID11+HARVESTDHI.log1000.0 step2 runTheMatrix-results/1000.0_RunMinBias2011A+RunMinBias2011A+TIER0+SKIMD+HARVESTDfst2+ALCASPLIT/step2_RunMinBias2011A+RunMinBias2011A+TIER0+SKIMD+HARVESTDfst2+ALCASPLIT.log1001.0 step2 runTheMatrix-results/1001.0_RunMinBias2011A+RunMinBias2011A+TIER0EXP+ALCAEXP+ALCAHARVDSIPIXELCALRUN1+ALCAHARVD1+ALCAHARVD2+ALCAHARVD3+ALCAHARVD4+ALCAHARVD5/step2_RunMinBias2011A+RunMinBias2011A+TIER0EXP+ALCAEXP+ALCAHARVDSIPIXELCALRUN1+ALCAHARVD1+ALCAHARVD2+ALCAHARVD3+ALCAHARVD4+ALCAHARVD5.log
I found errors in the following addon tests: cmsDriver.py RelVal -s L1REPACK:Full --data --scenario=pp -n 10 --conditions auto:run2_hlt_Fake2 --relval 9000,50 --datatier "RAW" --eventcontent RAW --customise=HLTrigger/Configuration/CustomConfigs.L1T --era Run2_2016 --fileout file:RelVal_Raw_Fake2_DATA.root --filein /store/data/Run2016B/JetHT/RAW/v1/000/272/762/00000/C666CDE2-E013-E611-B15A-02163E011DBE.root : FAILED - time: date Sat Oct 3 07:59:28 2020-date Sat Oct 3 07:59:20 2020 s - exit: 256 |
Comparison not run due to runTheMatrix errors (RelVals and Igprof tests were also skipped) |
So the 2 deprecated configurations I intended to cleanup are actually still relied on - I clearly overlooked this. |
These are, unfortunately, still in use. Will target them in a future PR.
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-31651/18765 |
@@ -0,0 +1,13 @@ | |||
### THIS CFF IS DEPRECATED!!!! ### | |||
# please use MagneticField_cff.py instead |
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.
In the comment fields you suggest to replace this with MagneticField_cff,
while in the command line you substitute it with MagneticField_38T_cff:
please fix the one which is wrong.
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.
It really depends what users intended to get when they picked this cff.
The name suggests a fixed 3.8T map regardless of conditions, so MagneticField_38T_cff would be the verbatim replacement. This is why it is substituted with MagneticField_38T_cff in the command.
But most likely it has been picked because it looked like the right one at the time: I expect that in 99% of the cases the standard MagneticField_cff would be the right choice instead. Which is why users are recommended to use the latter instead. Usage of non-conditions-dependent configs should in fact be discouraged for general usage.
BTW (although it's probably clear) I am just reverting here these files as they were before,
Eventually going through all instances and fixing them one by one with the correct cff will be the right choice, see comment below.
@namapane These two deprecated configs appear to be included in a few tests. One should eventually remove the two configs, and replace them with the correct one in all those tests. This can be longish, but easy to do: maybe it could be applied already once for all in this PR. |
Yeah I intend to replace them anywhere they appear, which deservers another PR. I just made a mistake here in searching it in dxr, and concluded it was no longer used, so safe to be bundled with other stuff... |
please test |
The tests are being triggered in jenkins.
|
+1 |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
+1
|
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
PR description:
Add standard MF regression tests as IB tests. Both DDD and DD4hep versions of the MF geometry builder are tested.
Also, cleanup 2 obsolete MF standard sequences that have been deprecated since ages and are no longer referenced anywhere.
PR validation:
Tested that the tests succeed in normal conditions and that regression failures are reported.