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

Changes to prepare the integration of the Tracker upgrade geometries #6947

Merged

Conversation

venturia
Copy link
Contributor

New GeomDetEnumerators and GeometricDet enumerators have been added to host the tracker upgrade subdetectors.
The building of the TrackerGeometry object is less hardcoded and depends on the enumerator values
New methods added to the TrackerGeometry object to provide, for each DetId subdetector index, the subdetector type (through the GeomDetEnumerators) and the number of layers in the subdetector. In addition the method "isThere" has been added to allow to check if a specific subdetector is in the geometry. This should allow to get rid of the several ad hoc configuration parameters to deal with the upgrade geometries.
The rules to code and decode the DetId schema are now completely configurable. Different detid schemas have been introduced for phase 1 and phase 2 geometries (in preparation for the integration of the upgrade geometries).
Added new CmsTracker…Builder's to ease the modularity with respect to the upgrade geometries. Now all the branching is based on the value of the GeometricDet enumerators.
The same approach has been followed for the construction of the TkDetLayers tree.
Replacement in many places of the checks on the DetId subset with checks on the GeomDetEnumerators (but the work is not finished…). Migration from the static DetId classes to the TrackerTopology class (but the work is FAR from being finished). Added useful static methods to discriminate on the GeomDetEnumerators: isTrackerPixel, isTrackerStrip, … and modified the implementation of the corresponding methods in GeomDetType to profit of them (the GeomDetType is not always available).
The present tag is expect not to change ANYTHING in the workflow output as the corresponding (still) unpublished changes for CMSSW_6_2_X_SLHC are expected not to change anything in the upgrade workflow (unless the version with the new phase 2 detid is used).

…ate GeometriDet tree and GeometricSearchTracker tree modified accordingly. More configurable tracker numbering builder and new classes for upgade subdetectors
…erTopologyEP and cleanup of the customization and reco Geometry configurations to profit of this
Merge cms-sw CMSSW_7_4_X into my trackergeo_readyforupgrade-74x
…d new static methods to discriminate according to the enumerators and use of these new static methods in the GeomDetType class
…ted a map between GeometricDet and GeomDetEnumerators enumerators and added ethods to TrackerGeometry to retrieve the subdetector type and the number of layers per subdetector
…between inner pixel and phase 2 outer tracker
…d schemas and updates of the configuration files to use them
…eparation between inner pixel and phase 2 outer tracker
@davidlange6
Copy link
Contributor

by-passing remaining signatures as changes there are small. Please complain if not ok.

davidlange6 added a commit that referenced this pull request Jan 5, 2015
Changes to prepare the integration of the Tracker upgrade geometries
@davidlange6 davidlange6 merged commit 9f4c4bf into cms-sw:CMSSW_7_4_X Jan 5, 2015
@@ -24,9 +24,30 @@ using namespace edm;

TrackerGeometricDetESModule::TrackerGeometricDetESModule( const edm::ParameterSet & p )
: fromDDD_( p.getParameter<bool>( "fromDDD" )),
layerNumberPXB_( p.exists( "layerNumberPXB" ) ? p.getParameter<unsigned int>( "layerNumberPXB" ) : 16U ),// 16 for current, 18 for SLHC
totalBlade_( p.exists( "totalBlade" ) ? p.getParameter<unsigned int>( "totalBlade" ) : 24U ) // 24 for current, 56 for SLHC
Copy link
Contributor

Choose a reason for hiding this comment

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

Removing these two lines, plus fillDescriptions checks, now break HLT:

----- Begin Fatal Exception 06-Jan-2015 07:29:23 CET-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Validating configuration of ESProducer or ESSource of type TrackerGeome\
tricDetESModule with label: 'TrackerGeometricDetESModule'
Exception Message:
Illegal parameters found in configuration.  The parameters are named:
 'layerNumberPXB'
 'totalBlade'
You could be trying to use parameter names that are not
allowed for this plugin or they could be misspelled.
----- End Fatal Exception -------------------------------------------------

I wonder why this was not flagged by jenkins tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

Because it is not tested --- sigh! I make a PR....

Copy link
Contributor

Choose a reason for hiding this comment

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

by "break the HLT" - what test do you mean exactly? We certainly do not have a mass failure in the IBs.

On Jan 6, 2015, at 8:04 AM, Martin Grunewald notifications@github.com
wrote:

In Geometry/TrackerNumberingBuilder/plugins/TrackerGeometricDetESModule.cc:

@@ -24,9 +24,30 @@ using namespace edm;

TrackerGeometricDetESModule::TrackerGeometricDetESModule( const edm::ParameterSet & p )
: fromDDD_( p.getParameter( "fromDDD" )),

  • layerNumberPXB_( p.exists( "layerNumberPXB" ) ? p.getParameter( "layerNumberPXB" ) : 16U ),// 16 for current, 18 for SLHC
  • totalBlade_( p.exists( "totalBlade" ) ? p.getParameter( "totalBlade" ) : 24U ) // 24 for current, 56 for SLHC

Because it is not tested --- sigh! I make a PR....


Reply to this email directly or view it on GitHub.

Copy link
Contributor

Choose a reason for hiding this comment

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

The HLT tests running the HLT the "online" way (fully self-contained cfg from ConfDB).
This is tested as part of the special HLT addOn tests, but not in the normal cmsDriver-
based IB tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In any case the fix should be such not to use those two parameters in the configurations anymore. I thought fixing the configurations in fillDescriptions was enough. Were are those parameters still used?

@venturia
Copy link
Contributor Author

venturia commented Jan 6, 2015

In any case the fix should be such not to use those two parameters in the configurations anymore. I thought fixing the configurations in fillDescriptions was enough. Were are those parameters still used?

@Martin-Grunewald
Copy link
Contributor

In ConfDB :) Indeed I'll remove these two, which should work both in 73X and 74X
because in 73X we still have the exitsAs fixup with the values as set in the HLT config
in ConfDB.

@Martin-Grunewald
Copy link
Contributor

fillDescriptions still will complain about "unknown" parameters;
I guess in such a case one could denote those parameters as optional
in fillDescriptions (and then not use them further in the code), but
this would mean carrying around cruft...

@Martin-Grunewald
Copy link
Contributor

... so it is bettter I remove them in ConfDb and make new HLT dumps!

@Martin-Grunewald
Copy link
Contributor

#7045 fixes the issue for HLT.

cmsbuild added a commit that referenced this pull request Jan 6, 2015
…oduleFIX

Fix HLT cfg dumps for TrackerGeometricDetESModule (fallout of #6947)
@venturia venturia deleted the trackergeo_readyforupgrade-74x branch September 9, 2016 07:18
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