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

add CaloTowerTopology cfi to HGCal geom configs #3229

Merged
merged 1 commit into from Apr 7, 2014

Conversation

kpedro88
Copy link
Contributor

@kpedro88 kpedro88 commented Apr 6, 2014

This fixes the CaloTowerTopology-related error in the HGCal tests as noted in pull request 3187.

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 6, 2014

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

add CaloTowerTopology cfi to HGCal geom configs

It involves the following packages:

Configuration/Geometry

@civanch, @Dr15Jones, @ianna, @mdhildreth, @cmsbuild, @nclopezo, @Degano, @ktf can you please review it and eventually sign? Thanks.
@ghellwig this is something you requested to watch as well.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.
@andersonjacob, @mark-grimes you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@mark-grimes
Copy link

I tried this after your suggestion, and now get

----- Begin Fatal Exception 05-Apr-2014 19:45:42 CEST-----------------------
An exception of category 'NoRecord' occurred while
   [0] Processing run: 1
   [1] Running path 'digitisation_step'
   [2] Calling beginRun for module EcalTrigPrimProducer/'simEcalTriggerPrimitiveDigis'
Exception Message:
No "EcalEndcapGeometryRecord" record found in the EventSetup.
 Please add an ESSource or ESProducer that delivers such a record.
----- End Fatal Exception -------------------------------------------------

Couldn't find an obvious fix. Any ideas? Happy to merge this though once I've double checked something, since it gets things one step closer.

@mark-grimes
Copy link

merge

HGCal scenarios still fail in step 2 with the previously mentioned error, but it's a step closer.

cmsbuild added a commit that referenced this pull request Apr 7, 2014
add CaloTowerTopology cfi to HGCal geom configs
@cmsbuild cmsbuild merged commit 740f180 into cms-sw:CMSSW_6_2_X_SLHC Apr 7, 2014
@kpedro88
Copy link
Contributor Author

kpedro88 commented Apr 7, 2014

I also get that error. I didn't know if that was the error with which the HGCal tests failed before my pull request 3187, but I guess it wasn't. The HGCal geometries appear to deliberately exclude the EE geometry (which would make sense). It looks like the EcalTrigPrimProducer configuration already has a setting BarrelOnly to handle this case. I added this configuration line to the end of the step2_DIGI_L1_DIGI2RAW.py config file produced by runTheMatrix:

process.simEcalTriggerPrimitiveDigis.BarrelOnly = cms.bool(True)

With this, I get a different error:

cmsRun: /build/cmsbuild/jenkins-workarea/workspace/ib-CMSSW_6_2_X_SLHC-slc5_amd64_gcc472/171/w/tmp/BUILDROOT/9bd31059797340bebc0312d2712077ce/opt/cmssw/slc5_amd64_gcc472/cms/cmssw-patch/CMSSW_6_2_X_SLHC_2014-04-05-1400/src/SimCalorimetry/EcalSimAlgos/src/ESDigitizer.cc:70: void ESDigitizer::setGain(int): Assertion `0 != m_detIds && 0 != m_detIds->size()' failed.

That configuration line should be included for the HGCal customizations, but I don't know where it ought to go. For this next error, there is probably some way to disable digitization for the preshower, but I'm not sure how.

@mark-grimes
Copy link

@kpedro88 the error we used to get was

cmsRun: <build path>/src/Geometry/CaloGeometry/interface/EZMgrFL.h:37: EZMgrFL<T>::iterator EZMgrFL<T>::assign(const T&) const [with T = float; EZMgrFL<T>::iterator = __gnu_cxx::__normal_iterator<float*, std::vector<float> >]: Assertion `( m_vec.size() + m_subSize ) <= m_vecSize' failed.

Presumably that is fixed by one of your pull requests (or we could hit it again later down the line). This error is similar to the one for SHCal:

cmsRun: <build path>/src/Geometry/EcalAlgo/src/EcalPreshowerGeometry.cc:136: virtual void EcalPreshowerGeometry::initializeParms(): Assertion `0 != n1minus && 0 != n2minus && 0 != n1plus && 0 != n2plus' failed.

That one appears to be a hard-coded check that EE is present. I considered going in and taking it out but that's a lot of tinkering in something I don't understand.
That "BarrelOnly" line will have to go in a HGCal customisation. I'll put it on my to-do list (unless you know where to add it in?).

@kpedro88
Copy link
Contributor Author

kpedro88 commented Apr 7, 2014

One of my pull requests may have impacted EZMgrFL (if it gets used in relation to the CaloTowerGeometry), but I'm not sure. It may pop up again later.

I don't know where the BarrelOnly line should go in the HGCal customizations. Maybe one of the HGCal people can tell us.

We probably need to get some ECAL-familiar developers to figure out how to get away with removing the preshower in the geometry configs.

@ianna
Copy link
Contributor

ianna commented Apr 8, 2014

@mark-grimes and @kpedro88 - BarrelOnly is defined in Reco part of geometry configuration. There are separate fragments added instead of one including everything:

https://github.com/cms-sw/cmssw/blob/CMSSW_6_2_X_SLHC/Configuration/Geometry/python/GeometryExtended2023HGCalReco_cff.py#L31

You have to be sure that it's not overwritten by other includes you've added.

@mark-grimes
Copy link

@ianna simEcalTriggerPrimitiveDigis is not defined until much later, so I think it will have to go in the customisation. Adding "simEcalTriggerPrimitiveDigis.BarrelOnly = cms.bool(True)" to the end of GeometryExtended2023HGCalReco_cff.py just gives

NameError: name 'simEcalTriggerPrimitiveDigis' is not defined

HGCal uses combinedCustoms.cust_2023 as does Extended2023 (SHCal does too but presumably that requires the same line added). Should I make a copy of cust_2023 to say cust_2023NoEE and add the line in there, then switch HGCal and SHCal to use that?

@ianna
Copy link
Contributor

ianna commented Apr 8, 2014

@mark-grimes - cust_2023NoEE seems clear enough. I remember, there was also a different plugin for when there is no ES. I'm not sure it is already in customization.

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

Successfully merging this pull request may close these issues.

None yet

4 participants