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

[WIP] First DECal / SiW ECal commit #294

Open
wants to merge 32 commits into
base: master
Choose a base branch
from
Open

Conversation

tonyPbA
Copy link

@tonyPbA tonyPbA commented Mar 7, 2018

Addresses # .

Changes proposed in this pull-request:

image

To be done:

  • [run scripts currently fail due to moving from v0.8 to v0.9. Currently working on this]
  • [ ]

Tony Price and others added 25 commits November 17, 2016 23:45
…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
@phsft-bot
Copy link

Can one of the admins verify this patch?

@tonyPbA tonyPbA changed the title First DECal / SiW ECal commit [WIP] First DECal / SiW ECal commit Mar 7, 2018
@zaborowska
Copy link
Contributor

ok to test

@phsft-bot
Copy link

Starting build on slc6/gcc62
How to customize builds

@phsft-bot
Copy link

All tests succeeded

@JavierCVilla
Copy link
Contributor

JavierCVilla commented Mar 12, 2018

@phsft-bot build please

@phsft-bot
Copy link

Starting build on slc6/gcc62
How to customize builds

@phsft-bot
Copy link

All tests succeeded

@phsft-bot
Copy link

Starting build on slc6/gcc62
How to customize builds

@phsft-bot
Copy link

Build failed on slc6/gcc62
See console output.

@phsft-bot
Copy link

Starting build on slc6/gcc62
How to customize builds

@phsft-bot
Copy link

Build failed on slc6/gcc62
See console output.

Failing tests:

@phsft-bot
Copy link

Starting build on slc6/gcc62
How to customize builds

@phsft-bot
Copy link

Build failed on slc6/gcc62
See console output.

Failing tests:

@phsft-bot
Copy link

Starting build on slc6/gcc62
How to customize builds

@phsft-bot
Copy link

Build failed on slc6/gcc62
See console output.

Failing tests:

@phsft-bot
Copy link

Starting build on slc6/gcc62
How to customize builds

@phsft-bot
Copy link

Build failed on slc6/gcc62
See console output.

Failing tests:

@phsft-bot
Copy link

Starting build on slc6/gcc62
How to customize builds

@phsft-bot
Copy link

Build failed on slc6/gcc62
See console output.

Failing tests:

Copy link
Contributor

@zaborowska zaborowska left a 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 -->
Copy link
Contributor

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 -->
Copy link
Contributor

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)");

Copy link
Contributor

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;
Copy link
Contributor

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"};

Copy link
Contributor

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};
Copy link
Contributor

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
Copy link
Contributor

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",
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants