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 PLT To Event Simulation #3087

Merged
merged 5 commits into from Mar 31, 2014
Merged

Conversation

skaplanhex
Copy link
Contributor

I think that I am still having problems with other volumes (perhaps my own) diminishing the number of PSimHits I see in the PLT. Can this be tested? Thanks!

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @skaplanhex (Steven Kaplan) for CMSSW_7_1_X.

Add PLT To Event Simulation

It involves the following packages:

SimG4CMS/Forward
SimGeneral/MixingModule

@cmsbuild, @civanch, @Degano, @mdhildreth, @nclopezo can you please review it and eventually sign? Thanks.
@wmtan 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.
@nclopezo, @ktf you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@cmsbuild
Copy link
Contributor

-1
I found an error when building:

>> Compiling  /build/cmsbuild/jenkins-workarea/workspace/ib-integration-CMSSW_7_1_X-slc6_amd64_gcc481/CMSSW_7_1_X_2014-03-31-0200/src/SimG4CMS/Forward/src/TotemT1Organization.cc 
>> Compiling  /build/cmsbuild/jenkins-workarea/workspace/ib-integration-CMSSW_7_1_X-slc6_amd64_gcc481/CMSSW_7_1_X_2014-03-31-0200/src/SimG4CMS/Forward/src/TotemT2NumberingSchemeGem.cc 
>> Compiling  /build/cmsbuild/jenkins-workarea/workspace/ib-integration-CMSSW_7_1_X-slc6_amd64_gcc481/CMSSW_7_1_X_2014-03-31-0200/src/SimG4CMS/Forward/src/TotemT2OrganizationGem.cc 
>> Compiling  /build/cmsbuild/jenkins-workarea/workspace/ib-integration-CMSSW_7_1_X-slc6_amd64_gcc481/CMSSW_7_1_X_2014-03-31-0200/src/SimG4CMS/Forward/src/TotemTestGem.cc 
/build/cmsbuild/jenkins-workarea/workspace/ib-integration-CMSSW_7_1_X-slc6_amd64_gcc481/CMSSW_7_1_X_2014-03-31-0200/src/SimG4CMS/Forward/src/PltSD.cc: In constructor 'PltSD::PltSD(std::string, const DDCompactView&, SensitiveDetectorCatalog&, const edm::ParameterSet&, const SimTrackManager_)':
/build/cmsbuild/jenkins-workarea/workspace/ib-integration-CMSSW_7_1_X-slc6_amd64_gcc481/CMSSW_7_1_X_2014-03-31-0200/src/SimG4CMS/Forward/src/PltSD.cc:41:98: error: 'GeV' was not declared in this scope
     energyCut           = m_TrackerSD.getParameter("EnergyThresholdForPersistencyInGeV")_GeV; //default must be 0.5 (?)
                                                                                                  ^
/build/cmsbuild/jenkins-workarea/workspace/ib-integration-CMSSW_7_1_X-slc6_amd64_gcc481/CMSSW_7_1_X_2014-03-31-0200/src/SimG4CMS/Forward/src/PltSD.cc:41:98: note: suggested alternative:
In file included from /afs/cern.ch/cms/sw/ReleaseCandidates/vol0/slc6_amd64_gcc481/external/geant4/10.00.p01/include/Geant4/CLHEP/Units/PhysicalConstants.h:42:0,
                 from /afs/cern.ch/cms/sw/ReleaseCandidates/vol0/slc6_amd64_gcc481/external/geant4/10.00.p01/include/Geant4/CLHEP/Vector/RotationX.icc:11,


you can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3087/715/summary.html

@civanch
Copy link
Contributor

civanch commented Mar 31, 2014

@skaplanhex, to fix the problem you need to add header file:

"#include "CLHEP/Units/SystemOfUnits.h"

This is needed after we switch to Geant4 10.0.

Vladimir

@skaplanhex
Copy link
Contributor Author

@civanch I added the header. Thanks! I tried to compile my area, but I was having compilation issues with an unrelated file. I think it should be ok though.

@cmsbuild
Copy link
Contributor

Pull request #3087 was updated. @cmsbuild, @civanch, @Degano, @mdhildreth, @nclopezo can you please check and sign again.

@civanch
Copy link
Contributor

civanch commented Mar 31, 2014

Sorry, now I compile myself and you need

#include "CLHEP/Units/GlobalSystemOfUnits.h"

instead of #include "CLHEP/Units/SystemOfUnits.h"

Vladimir

@cmsbuild
Copy link
Contributor

Pull request #3087 was updated. @cmsbuild, @civanch, @Degano, @mdhildreth, @nclopezo can you please check and sign again.

@cmsbuild
Copy link
Contributor

Pull request #3087 was updated. @cmsbuild, @civanch, @Degano, @mdhildreth, @nclopezo can you please check and sign again.

@civanch
Copy link
Contributor

civanch commented Mar 31, 2014

+1

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_1_X IBs unless changes or unless it breaks tests. @nclopezo, @ktf can you please take care of it?

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_1_X IBs unless changes (tests are also fine). @nclopezo, @ktf can you please take care of it?

@@ -35,6 +36,7 @@
#crossingFrames = cms.untracked.vstring(
# 'BSCHits',
# 'BCM1FHits',
# 'PLTHits'
Copy link
Contributor

Choose a reason for hiding this comment

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

I would recommend removing commented code. In this particular case the change you is also wrong (missing comma). Please remove this completely in a separate pull request.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ktf OK, I will open the PR in a few minutes. Thank you for the suggestion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ktf Done. The pull request can be found here: #3119. Thanks!

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_1_X IBs unless changes (tests are also fine). @nclopezo, @ktf can you please take care of it?

ktf added a commit that referenced this pull request Mar 31, 2014
@ktf ktf merged commit 9e9377d into cms-sw:CMSSW_7_1_X Mar 31, 2014
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