Additions for the GEM SliceTest #16900
Conversation
|
A new Pull Request was created by @calabria (Cesare) for CMSSW_9_0_X. It involves the following packages: Configuration/Eras @smuzaffar, @civanch, @Dr15Jones, @dmitrijus, @cvuosalo, @ianna, @mdhildreth, @fabozzi, @cmsbuild, @rekovic, @srimanob, @franzoni, @slava77, @vanbesien, @hengne, @mulhearn, @davidlange6 can you please review it and eventually sign? Thanks. cms-bot commands are listed here #13028 |
| from Configuration.Eras.Modifier_run2_HCAL_2017_cff import run2_HCAL_2017 | ||
| from Configuration.Eras.Modifier_run2_GEMSliceTest_cff import run2_GEMSliceTest | ||
|
|
||
| Run2_2017Muon = cms.ModifierChain(run2_2017_core, trackingPhase1, run2_GEMSliceTest) |
There was a problem hiding this comment.
You could save some characters with
Run2_2017Muon = cms.ModifierChain(Run2_2017, run2_GEMSliceTest)
and be safe in case we change the default tracking era from trackingPhase1 to something else (ok, we won't).
| @@ -20,6 +20,7 @@ def __init__(self): | |||
| 'Run2_2016_trackingLowPU', | |||
| 'Run2_2016_pA', | |||
| 'Run2_2017', | |||
| 'Run2_2017Muon', | |||
There was a problem hiding this comment.
I believe you should add run2_GEMSliceTest to internalUseMods below (but I can't place this comment there).
|
@cmsbuild please test |
|
The tests are being triggered in jenkins. |
|
Comparison job queued. |
| from Configuration.Eras.Era_Run2_2017_cff import Run2_2017 | ||
| from Configuration.Eras.Modifier_run2_GEMSliceTest_cff import run2_GEMSliceTest | ||
|
|
||
| Run2_2017Muon = cms.ModifierChain(Run2_2017, run2_GEMSliceTest) |
There was a problem hiding this comment.
Why not just call this uniformly Run2_2017GEM and the internal scenario run2_GEM?
There was a problem hiding this comment.
for me it makes no difference, I had to give a name and I did it :D if you think it is worth to change I will
|
Comparison is ready The workflows 1003.0, 1001.0, 1000.0, 140.53, 136.731, 4.22 have different files in step1_dasquery.log than the ones found in the baseline. You may want to check and retrigger the tests if necessary. You can check it in the "files" directory in the results of the comparisons |
|
+1 |
cmsbuild
added
alca-pending
analysis-pending
db-pending
l1-pending
reconstruction-pending
and removed
operations-pending
pdmv-pending
upgrade-pending
labels
|
please test |
|
The tests are being triggered in jenkins. |
|
-1 Tested at: 50527a4 The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: You can see the results of the tests here: I found follow errors while testing this PR Failed tests: Build
I found an error when building: gmake[1]: Target 'PostBuild' not remade because of errors. gmake[1]: Leaving directory '/build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_0_X_2017-02-08-1100' config/SCRAM/GMake/Makefile.rules:2017: recipe for target 'src' failed gmake: *** [src] Error 2 gmake: Target 'all' not remade because of errors. gmake: *** [There are compilation/build errors. Please see the detail log above.] Error 2 The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: |
|
Comparison not run due to Build errors (RelVals and Igprof tests were also skipped) |
|
no please wait, I have not been able to solve the problems with git. |
cmsbuild
added
comparison-notrun
tests-rejected
and removed
comparison-pending
tests-started
labels
|
@calabria |
|
the error in the last build was seems like something easy to fix |
|
@slava77 thanks. yes I noticed it and fixed it... But I am preparing a new branch, I don't think I can continue with this one... Unfortunately the validation packages are now crashing for some reason and I am trying to understand why... |
|
hello @calabria |
|
Hi @slava77 yes sure, but unfortunately we have another issue to solve: we are having problems with the GEMGeometry taken from the Database, when asking the regions of the Geometry we get a crash. Maybe the geometry constructed from the database is not as complete as the geometry we can take from DDD. We realized the part where chambers, rings, stations and regions are created is missing in the geometryFromCondDB. We have to work on an increased description of the CondDB geometry first. We already contacted @ianna to get some help on how to proceed because it is not perfectly clear: we need to know how we can test, if we need to update the payload... |
|
@ianna the problem shows up in the validation, if I run the simulation up to the reconstruction stepeverything "seems" to work fine. So in order to reproduce the crash it is enough if you run the reco step + validation, so if you run 10411 you should see the problem. But you need the modifications of this PR, but I don't think you can merge use this branch. I have a clean one: calabria:New2017Muon6 |
|
@calabria - thanks, I can reproduce the crash: The vector of GEMRegions is empty, so the code accessing its constituents is not safe. GEM geometry builder from DB constructs only Eta partitions (80 of them in this particular case): http://cmslxr.fnal.gov/source/Geometry/GEMGeometryBuilder/src/GEMGeometryBuilderFromCondDB.cc#0033 Usually, RECO geometry contains only sensitive volumes, however, if GEM reco geometry needs to provide more (regions, superChambers and chambers) GEMGeometryBuilderFromCondDB is the place to add these. Please, let me know if you need my help in implementing the code. |
This PR introduces the needed changes to have a 2017 scenario with the GEM chambers foreseen for the next year slice test. We define a specific scenario, Extended2017Muon, where the simulation/reconstruction stops at local reco. We would like to have this in 900_pre2, because it is enough for the moment to be able to validate the new scenario with the relVal production foreseen for that pre-release, even though we don't enter the default 2017 scenario.
@jshlee @pietverwilligen