-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Phase2-hgx128 Complete the SIM steps for HFNose #23794
Conversation
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-23794/5487 |
A new Pull Request was created by @bsunanda for master. It involves the following packages: Geometry/HGCalCommonData @andrius-k, @Dr15Jones, @kmaeshima, @schneiml, @ianna, @mdhildreth, @dmitrijus, @cmsbuild, @jfernan2, @kpedro88, @civanch can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
please test |
The tests are being triggered in jenkins. |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
#include "Geometry/HGCalCommonData/interface/HGCalDDDConstants.h" | ||
#include "Geometry/HGCalCommonData/interface/HGCalGeometryMode.h" | ||
|
||
#include "G4Step.hh" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this include needed?
#endif | ||
} | ||
#ifdef EDM_ML_DEBUG | ||
edm::LogVerbatim("HGCSim") << "HGCalNumberingScheme::i/p " << layer << ":" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change "HGCSim", "HGCalNumberingScheme" to "HFNoseSim", "HFNoseNumberingScheme" to avoid confusion
SimG4CMS/Calo/src/HFNoseSD.cc
Outdated
<< "\n" | ||
<< "**************************************************"; | ||
#endif | ||
edm::LogVerbatim("HGCSim") << "HFNoseSD:: Threshold for storing hits: " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should these all be inside the EDM_ML_DEBUG
ifdef?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer to keep it out of EDM_ML_DEBUG
@cmsbuild Please test |
Comparison is ready Comparison Summary:
|
+upgrade |
@ianna @civanch @dmitrijus Could you approve this PR so that we can go forward to complete Workflow with HFNose |
+1 |
@civanch @dmitrijus Could you please sign this PR |
+1 |
@fabiocos @dmitrijus I think only signature from DQM is missing. This has a small change in the validation code to test geometry of HFNose - could this be signed and merged urgently? |
@jfernan2 could dqm please have a look? |
+1
…________________________________________
From: Fabio Cossutti [notifications@github.com]
Sent: 18 July 2018 21:01
To: cms-sw/cmssw
Cc: Javier Fernandez Menendez; Mention
Subject: Re: [cms-sw/cmssw] Phase2-hgx128 Complete the SIM steps for HFNose (#23794)
@jfernan2<https://github.com/jfernan2> could dqm please have a look?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#23794 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AETJ6VHsvluKL82Bz985Gi_n6P-zF3yfks5uH4YigaJpZM4VPeIr>.
|
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
Now one can run simulation in a geometry scenario with HFNose and can get the PCaloHit collection for HFNose