gem geometry to DB writer #21307
gem geometry to DB writer #21307
Conversation
|
The code-checks are being triggered in jenkins. |
|
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/PR-21307/1994 Code check has found code style and quality issues which could be resolved by applying a patch in https://cmssdt.cern.ch/SDT/code-checks/PR-21307/1994/git-diff.patch You can run |
|
The code-checks are being triggered in jenkins. |
|
+code-checks |
|
A new Pull Request was created by @jshlee (Jason Lee) for master. It involves the following packages: CondTools/Geometry @civanch, @Dr15Jones, @ianna, @mdhildreth, @cmsbuild, @kpedro88, @ggovi can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
| if (DDfetch( *is, numbOfPads)) nPads = numbOfPads.doubles()[0]; | ||
| } | ||
| LogDebug("GEMGeometryParsFromDD") | ||
| << ((nStrips == 0. ) ? ("No nStrips found!!") : ("Number of strips: " + boost::lexical_cast<std::string>(nStrips))); |
There was a problem hiding this comment.
@jshlee - please, use std::to_string
| LogDebug("GEMGeometryParsFromDD") | ||
| << ((nStrips == 0. ) ? ("No nStrips found!!") : ("Number of strips: " + boost::lexical_cast<std::string>(nStrips))); | ||
| LogDebug("GEMGeometryParsFromDD") | ||
| << ((nPads == 0. ) ? ("No nPads found!!") : ("Number of pads: " + boost::lexical_cast<std::string>(nPads))); |
There was a problem hiding this comment.
@jshlee - please, use std::to_string
| @@ -20,6 +20,7 @@ | |||
|
|
|||
| #include <iostream> | |||
| #include <algorithm> | |||
| #include <boost/lexical_cast.hpp> | |||
There was a problem hiding this comment.
@jshlee - please, remove dependency on boost, prefer to use the standard library instead.
| @@ -1,8 +1,8 @@ | |||
| import FWCore.ParameterSet.Config as cms | |||
There was a problem hiding this comment.
@jshlee - this one suppose to test ME0 only. The test for GEM is this one: https://cmssdt.cern.ch/lxr/source/CondTools/Geometry/test/gemgeometrywriter.py, but the actual DB writers are in test/writehelpers directory. I suggest, you revert modifications of this file, e.g. it is not needed in this PR. Thanks.
| std::string name = fv.logicalPart().name().name(); | ||
| strpars.emplace_back(name); | ||
|
|
||
| std::vector<double> pars; |
There was a problem hiding this comment.
@jshlee - please, use list initialization:
- std::vector<double> pars;
- pars.emplace_back(dx1); //b/2; bottom width is along local X
- pars.emplace_back(dx2); //B/2; top width is along local X
- pars.emplace_back(dy); //h/2; length is along local Y
- pars.emplace_back(dz); // thickness is long local Z
+ std::vector<double> pars {dx1, dx2, dy, dz};
| std::string name = fv.logicalPart().name().name(); | ||
| strpars.emplace_back(name); | ||
|
|
||
| std::vector<double> pars; |
There was a problem hiding this comment.
@jshlee - please, use list initialization here
| std::string name = fv.logicalPart().name().name(); | ||
| strpars.emplace_back(name); | ||
|
|
||
| std::vector<double> pars; |
There was a problem hiding this comment.
@jshlee - please, use list initialization here
| vtra[0]=(float) 1.0 * (tran.x()); | ||
| vtra[1]=(float) 1.0 * (tran.y()); | ||
| vtra[2]=(float) 1.0 * (tran.z()); | ||
| return vtra; |
There was a problem hiding this comment.
@jshlee - please, use list initialization in return statement here
| vrot[6]=(float) 1.0 * z.X(); | ||
| vrot[7]=(float) 1.0 * z.Y(); | ||
| vrot[8]=(float) 1.0 * z.Z(); | ||
| return vrot; |
There was a problem hiding this comment.
@jshlee - please, use list initialization in return statement here
| } | ||
| LogDebug("ME0GeometryParsFromDD") | ||
| << ((nStrips == 0. ) ? ("No nStrips found!!") : ("Number of strips: " + boost::lexical_cast<std::string>(nStrips))); |
There was a problem hiding this comment.
@jshlee - please, use std::to_string
|
The code-checks are being triggered in jenkins. |
|
+code-checks |
|
The code-checks are being triggered in jenkins. |
|
please test |
|
+code-checks |
|
The tests are being triggered in jenkins. |
|
Pull request #21307 was updated. @civanch, @Dr15Jones, @ianna, @mdhildreth, @cmsbuild, @kpedro88, @ggovi can you please check and sign again. |
|
+1 The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: |
|
Comparison job queued. |
|
Comparison is ready Comparison Summary:
|
|
+1 |
|
+1 |
|
merge |
@ianna