-
Notifications
You must be signed in to change notification settings - Fork 112
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
[WIP] First DECal / SiW ECal commit #294
base: master
Are you sure you want to change the base?
Conversation
…added script to run a simulation then reconstruct
… This generates the tree for statisics analysis. Also changed syntax for xml file naming to make easier to submit different geometry from single top level file. Setup file for SiW analogue readout with 300um of Silicon as sensitive layer, 50 layers of 2.1mm W
…on digital hits. Default values still work with other Calorimeters
Conflicts: Detector/DetSensitive/src/SDWrapper.cpp
…d code ready for PR
Can one of the admins verify this patch? |
ok to test |
Starting build on |
All tests succeeded |
@phsft-bot build please |
Starting build on |
All tests succeeded |
Starting build on |
Build failed on |
Starting build on |
Build failed on Failing tests: |
Starting build on |
Build failed on Failing tests: |
Starting build on |
Build failed on Failing tests: |
Starting build on |
Build failed on Failing tests: |
Starting build on |
Build failed on Failing tests: |
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.
Hi Tony!
thanks a lot for your contribution!
I have some comments, but most of them will be addressed if you run checkformat
on changed/new files (but it is important to commit other changes first, do the checkformat, and commit those changes, otherwise it may be difficult for me to follow relevant updates).
Also, a clean-up of all the python configs is rather necessary. And updates of the doxygen comments, so it is actually easier to understand what is the purpose of each file.
Thanks!
Anna
<comment>Envelope for HCal barrel</comment> | ||
<dimensions rmin="BarHCal_rmin+HcalSpacer" rmax="BarHCal_rmax" dz="BarHCal_dz" phi0="0" deltaphi="360*deg" z_offset="0*cm" material="Air"/> | ||
</detector> | ||
<!-- End-caps --> |
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.
If you need it just for the barrel, remove the commented out code.
<comment>Envelope for Tracker</comment> | ||
<dimensions rmin="Tracker_rmin" rmax="Tracker_rmax" dz="Tracker_dz" phi0="0" deltaphi="360*deg" z_offset="0*cm" material="Air"/> | ||
</detector> | ||
<!-- Forward detectors --> |
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.
Same as above, remove the commented out code.
@@ -17,6 +17,7 @@ DECLARE_ALGORITHM_FACTORY(RedoSegmentation) | |||
RedoSegmentation::RedoSegmentation(const std::string& aName, ISvcLocator* aSvcLoc) : GaudiAlgorithm(aName, aSvcLoc) { | |||
declareProperty("inhits", m_inHits, "Hit collection with old segmentation (input)"); | |||
declareProperty("outhits", m_outHits, "Hit collection with modified segmentation (output)"); | |||
|
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.
If you do changes to the file, please run checkformat
afterwards.
The recommended way is to commit all your changes and then do checkformat -i <NAME_OF_CHANGED_FILE>
and commit again.
@@ -107,6 +114,7 @@ StatusCode RedoSegmentation::execute() { | |||
debugIter++; | |||
} | |||
} | |||
info() << "nhits = " << nhits << endmsg; |
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.
That probably should be in debug()
or I'd suggest even verbose()
to avoid unnecessary printouts (or maybe even delete this line?)
@@ -83,5 +83,6 @@ class RedoSegmentation : public GaudiAlgorithm { | |||
std::vector<std::string> m_detectorIdentifiers; | |||
/// Limit of debug printing | |||
Gaudi::Property<uint> m_debugPrint{this, "debugPrint", 10, "Limit of debug printing"}; | |||
|
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.
That should disappear if you run checkformat on that file.
/// Handle for calo hits (input collection) | ||
DataHandle<fcc::CaloHitCollection> m_hits{"hits", Gaudi::DataHandle::Reader, this}; | ||
/// Handle for calo cells (output collection) | ||
DataHandle<fcc::CaloHitCollection> m_cells{"cells", Gaudi::DataHandle::Writer, this}; | ||
DataHandle<fcc::CaloHitCollection> m_cells{"cellsm", Gaudi::DataHandle::Writer, this}; |
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.
Why cellsm
?
@@ -69,6 +69,13 @@ StatusCode SimG4SaveCalHits::saveOutput(const G4Event& aEvent) { | |||
auto edmHit = edmHits->create(); | |||
auto& edmHitCore = edmHit.core(); | |||
edmHitCore.cellId = hit->cellID; | |||
// added by Tony Price for timing |
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.
not necessary, github can tell us that ;)
fieldNames=["system"], | ||
fieldValues=[0], | ||
volumeMatchName="BoxECal", | ||
cells = TestCellCounting("cells", |
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.
Does it work now?
@@ -0,0 +1,267 @@ | |||
from ROOT import TH1F, TTree, TChain, TFile, TF1, TH2F, TGraphErrors, TCanvas, TPaveText |
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.
That file should not be in the main directory. Please clean it up, comment as much as possible, remove the paths to your code (any possible inputs like e.g. file names can be entered as arguments see here). This could probably go to Detector/DetectorFCChhECalDigital/scripts/
.
@@ -0,0 +1,132 @@ | |||
from ROOT import TH1F, TTree, TChain, TFile, TF1, TCanvas, TGraphErrors, TPaveText |
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.
same as above. Do you by any chance mean analogue when you name files SiW? I find it quite misleading, as digital is SiW as well, isn't it? (or Pb)
Addresses # .
Changes proposed in this pull-request:
To be done: