AIDASoft / DD4hep

Detector Description Toolkit for High Energy Physics
http://dd4hep.cern.ch
GNU Lesser General Public License v3.0
50 stars 99 forks source link

question about SDActions in DDSim steering file #819

Closed saraheno closed 3 years ago

saraheno commented 3 years ago

Hi,

I am using a detector written by Sang-hyun Ko (https://github.com/SanghyunKo/dual-readout/tree/master) for a dual readout spaghetti calorimeter.

In his code to define the geometry, he has a line like

DRgeo.cpp: sensDet.setType("DRcaloSiPMSD");

where the SD action for this is defined in his code https://github.com/SanghyunKo/dual-readout/tree/master/Detector/DRsensitive

I think I need a line in my DDSim steering file like

SIM.action.calo = "DualCrystalCalorimeterSDAction" to load the code for the DRcaloSiPMSD.

In my own code, I have a line like sens.setType("calorimeter").

but I am not sure what maps "calorimeter" to "calo" in the steering file. Would the right line be something like

SIM.action.DRcaloSiPMSD = "DRcaloSiPMSD"?

I note in his code SDWrapper.cpp, he has DECLARE_EXTERNAL_GEANT4SENSITIVEDETECTOR(DRcaloSiPMSD,dd4hep::sim::create_DRcaloSiPM_sd)

but I'm not sure how to map that onto a Steering file command...

andresailer commented 3 years ago

This is the 'calorimeter' that maps to the default action defined in self.action.calo

https://github.com/saraheno/DD4hep/blob/8a20b921e52a5ffdb489ccc613f7859bdcf35b80/examples/DDDualCrystal/SCEPCdetector/SCEPCALsteering.py#L67-L68

## List of patterns matching sensitive detectors of type Calorimeter.
SIM.action.calorimeterSDTypes = [u'calorimeter']

To set the DRcaloSiPMSD regardless of what is in sens.setType use

SIM.action.mapActions['NameInTheXMLTagOfTheDetector'] = 'DRcaloSiPMSD'
saraheno commented 3 years ago

When I just add that line to my SteeringFile, it does not work and I get

DDSim            INFO getDetectorLists - found active detector CrystalEcalBarrel type: calorimeter
DDSim            INFO getDetectorLists - found active detector DRcalo type: DRcaloSiPMSD
DDSim            WARNING Unknown sensitive detector type: DRcaloSiPMSD

and no hit information appears for DRcalo in the output root file

I also tried change in DRgeo.cpp sensDet.setType("DRcaloSiPMSD") to sensDet.setType("calorimeter") and adding that line as well. Then it finds it but crashes

DDSim            INFO Setting up SD for CrystalEcalBarrel
DDSim            INFO        replace default action with : DualCrystalCalorimeterSDAction
Geant4UI         INFO  +++ ParticleHandler> Install Geant4 control directory:/ddg4/Particl\
eHandler/
Geant4UI         INFO  +++ CrystalEcalBarrel> Install Geant4 control directory:/ddg4/Cryst\
alEcalBarrel/
DDSim            INFO Setting up SD for DRcalo
DDSim            INFO        replace default action with : DRcaloSiPMSD
Geant4UI         INFO  +++ CrystalEcalBarrelHandler> Install Geant4 control directory:/ddg\
4/CrystalEcalBarrelHandler/
Geant4UI         INFO  +++ DRcalo> Install Geant4 control directory:/ddg4/DRcalo/
Geant4Handle<Geant4Sensitive> ERROR Failed to create sensitive object of type DRcaloSiPMSD\
/DRcaloHandler for detector DRcalo!
DDSim            ERROR Setting up sensitive detector static dd4hep::sim::SensitiveHandle d\
d4hep::sim::Geant4ActionCreation::createSensitive(dd4hep::sim::KernelHandle& kernel, const\
 string& name_type, const string& detector, bool shared) =>
    runtime_error: Geant4Handle<Geant4Sensitive>: Failed to create sensitive object of typ\
e DRcaloSiPMSD/DRcaloHandler for detector DRcalo!
Traceback (most recent call last):
  File "/home/eno/dd4hep/DD4hep/bin/ddsim", line 25, in <module>
    RUNNER.run()
andresailer commented 3 years ago

Which line did you add? Could you link to your steering file?

saraheno commented 3 years ago

https://github.com/saraheno/DD4hep/blob/master/examples/DDDualCrystal/SCEPCdetector/SCEPCALsteering.py

andresailer commented 3 years ago

This line SIM.action.mapActions = {} overwrites what you set before. https://github.com/saraheno/DD4hep/blob/c03bfe4a5c41fc4f5d21457d778d57071c7e7bef/examples/DDDualCrystal/SCEPCdetector/SCEPCALsteering.py#L69-L75

saraheno commented 3 years ago

Unfortunately even after moving that, I get

Compact INFO ++ Converted subdetector:DRcalo of type ddDRcalo [DRcaloSiPMSD]

DDG4 INFO +++ DRcalo type:DRcaloSiPMSD --> Sensitive type: Unknown

DDSim.Helper.Filter INFO ReqFilt {'edep1kev'} DDSim INFO getDetectorLists - found active detector CrystalEcalBarrel type: calorimeter DDSim INFO getDetectorLists - found active detector DRcalo type: DRcaloSiPMSD DDSim WARNING Unknown sensitive detector type: DRcaloSiPMSD DDSim INFO Setting up SD for CrystalEcalBarrel DDSim INFO replace default action with : DualCrystalCalorimeterSDAction

I have updated the file in github with what I just ran

I also changed sensDet.setType back to sensDet.setType("DRcaloSiPMSD" in DRgeo.cpp

andresailer commented 3 years ago

Ok, at the moment you need to add this (and the mapActions stuff already there)

SIM.action.calorimeterSDTypes = [u'calorimeter', 'drcalosipmsd']

For that one to be picked up. Should add the unknowns to be tried against the mapped actions regardless of the calo/tracker type things.

But then you are back to the failure to load the DRcaloSiPMSD, which doesn't seem to be the kind of object expected for the sensitive detector types.

I don't know what the differences are between DECLARE_EXTERNAL_GEANT4SENSITIVEDETECTOR and DECLARE_GEANT4SENSITIVE, but I would think that they important.

saraheno commented 3 years ago

So when I just added the sim.action.calorimeterSDTypes line, it crashed with

DDSim INFO getDetectorLists - found active detector CrystalEcalBarrel \ type: calorimeter DDSim INFO getDetectorLists - found active detector DRcalo type: DRcal\ oSiPMSD DDSim INFO Setting up SD for CrystalEcalBarrel DDSim INFO replace default action with : DualCrystalCalorimeter\ SDAction Geant4UI INFO +++ ParticleHandler> Install Geant4 control directory:/ddg\ 4/ParticleHandler/ Geant4UI INFO +++ CrystalEcalBarrel> Install Geant4 control directory:/d\ dg4/CrystalEcalBarrel/ DDSim INFO Setting up SD for DRcalo DDSim INFO replace default action with : DRcaloSiPMSD Geant4UI INFO +++ CrystalEcalBarrelHandler> Install Geant4 control direc\ tory:/ddg4/CrystalEcalBarrelHandler/ Geant4UI INFO +++ DRcalo> Install Geant4 control directory:/ddg4/DRcalo/ Geant4Handle ERROR Failed to create sensitive object of type DRc\ aloSiPMSD/DRcaloHandler for detector DRcalo! DDSim ERROR Setting up sensitive detector static dd4hep::sim::Sensitiv\ eHandle dd4hep::sim::Geant4ActionCreation::createSensitive(dd4hep::sim::KernelHan\ dle& kernel, const string& name_type, const string& detector, bool shared) => runtime_error: Geant4Handle: Failed to create sensitive obje\ ct of type DRcaloSiPMSD/DRcaloHandler for detector DRcalo! Traceback (most recent call last): File "/home/eno/dd4hep/DD4hep/bin/ddsim", line 25, in RUNNER.run()

When I try to charnge DECLARE_EXTERNAL to just DECLARE, it won't compile

/home/eno/dd4hep/DD4hep/examples/DDDualCrystal/src/DRcaloSDWrapper.cpp:16:79: error: macro "DECLARE_GEANT4SENSITIVEDETECTOR" passed 2 arguments, but takes just 1 16 | E_GEANT4SENSITIVEDETECTOR(DRcaloSiPMSD,dd4hep::sim::create_DRcaloSiPM_sd) | ^

In file included from /home/eno/dd4hep/DD4hep/examples/DDDualCrystal/src/DRcaloSDWrapper.cpp:4: /home/eno/dd4hep/DD4hep/DDG4/include/DDG4/Factories.h:152: note: macro "DECLARE_GEANT4SENSITIVEDETECTOR" defined here 152 #define DECLARE_GEANT4SENSITIVEDETECTOR(id) __IMPLEMENT_GEANT4SENSDET(id,new id)

/home/eno/dd4hep/DD4hep/examples/DDDualCrystal/src/DRcaloSDWrapper.cpp:16:1: error: ‘DECLARE_GEANT4SENSITIVEDETECTOR’ does not name a type 16 | DECLARE_GEANT4SENSITIVEDETECTOR(DRcaloSiPMSD,dd4hep::sim::create_DRcaloSiPM_sd) | ^~~~~~~ /home/eno/dd4hep/DD4hep/examples/DDDualCrystal/src/DRcaloSDWrapper.cpp:8:32: warning: ‘G4VSensitiveDetector dd4hep::sim::create_DRcaloSiPM_sd(const string&, dd4hep::Detector&)’ defined but not used [-Wunused-function] 8 | static G4VSensitiveDetector create_DRcaloSiPM_sd(const std::string& aDetectorName, dd4hep::Detector& aLcdd) { | ^~~~~~~~ make[2]: [examples/DDDualCrystal/CMakeFiles/DRcaloSiPM.dir/src/DRcaloSDWrapper.cpp.o] Error 1 make[1]: [examples/DDDualCrystal/CMakeFiles/DRcaloSiPM.dir/all] Error 2 make: *** [all] Error 2

saraheno commented 3 years ago

so if I put a cout above https://github.com/saraheno/DD4hep/blob/master/examples/DDDualCrystal/src/DRcaloSDWrapper.cpp#L9

when would I expect this to be called? When is the DECLARE_EXTERNAL_GEANT4SENSTIVIEDETECTOR command executed? There must be something in the steering that triggers this to be created? or is it during the build? but when I rebuilt, I did not see the output from the cout I put there (or when I tried running ddsim either)

andresailer commented 3 years ago

This line 9 doesn't look like what I would understand as a cout.

saraheno commented 3 years ago

yes, I didn't commit the version with the cout to github. That is why I said "if I put". I guess more formally it should be "If I were to put"

I am still stuck on this, though. I cannot seem to get it to attach to the sensitive detector in Sang-hyun Ko's code. Any help would be appreciated.

saraheno commented 3 years ago

so anyway, when I do put a cout there, I do not see it in the spew. So this routine is not being called, and the object is not being created. I assume that the line in the steering file SIM.action.mapActions['DRcalo'] = 'DRcaloSiPMSD' is supposed to call this creator? But it cannot figure how this is the creator to call from this line? Is there anyway I can get ddsim to list the sensitive actions it knows about?

andresailer commented 3 years ago

The problem is that the object (DRcaloSiPMSD) is of the wrong type. It should be a Geant4SensitiveAction and not a G4VSensitiveDetector.

saraheno commented 3 years ago

I see. In his code, Sang-Hyuan only makes a GEANT4SENSITIVEDETECTOR. He doesn't make a GEANT4SENSITIVE. Can his version be used with ddsim? Or somehow I need to convert his GEANT4SENSITIVEDETECTOR to a GEANT4SENSITIVE?

MarkusFrankATcernch commented 3 years ago

GEANT4SENSITIVEDETECTOR is discontinued since roughly a month and was removed from the repository. It was deprecated since years and only kept for some time to be compatible with FCC until they remove the dependency.

saraheno commented 3 years ago

Is it easy to explain how to convert from SENSTIVIEDETECTOR to SENSITIVE? Or I should just compare the two codes and try to figure it out?

MarkusFrankATcernch commented 3 years ago

This should be very simple. Have a look in DDG4/plugins/Geant4SDActions.cpp

You effectively move the code from:

/// Method for generating hit(s) using the information of G4Step object.
virtual bool Geant4SensitiveDetector::buildHits(G4Step* /* step */,G4TouchableHistory* /* history */) 

to:

/// Method for generating hit(s) using the information of G4Step object.
template <> bool
Geant4SensitiveAction<MySDType>::process(G4Step* step,G4TouchableHistory* /* hist */) { ....

where MySDType is a class aggregated to a member m_userData this specialized version of Geant4SensitiveAction. The definition of MySDType is entirely up to you. In the simplest case it is empty:

class MySDType {
public:
  typedef Geant4Tracker::Hit Hit; // Or whatever you will output
};

and only satisfies the compiler.

Besides, you have to define this function to declare your output:

    /// Define collections created by this sensitivie action object
    template <> void Geant4SensitiveAction<Geant4Tracker>::defineCollections()    {
      m_collectionID = declareReadoutFilteredCollection<MySDType::Hit>();
    }

which is required by the G4VSensitiveDetector interface.

This in principle is all for a simple case. The logic to create hits is in Geant4SensitiveAction::process.

saraheno commented 3 years ago

I may need to restructure his hit definition to make this work. My hit definition inherits from Geant4Calorimeter::Hit, which inherits from Geant4HitData

his is like class DRcaloSiPMHit : public G4VHit

Is that G4VHit deprecated as well?

MarkusFrankATcernch commented 3 years ago

The Geant4Sensitive and the used data definition lifts off the dependency from G4VHit. You can directly use the hit class you want to store to file and clearly there you probably do not want to have a dependency on G4VHit which would compromise later code in digitization, reconstruction etc. Geant4HitData is only an example: you can use any other class as long as your Geant4Sensitive understands it - which it must since it creates these data.

saraheno commented 3 years ago

let me make sure I understand this. My main goal is to produce a root file with hit information for summer students to use in their projects. Right now, by making my DualCrystalCalorimeterHit class (https://github.com/saraheno/DD4hep/tree/master/examples/DDDualCrystal/src) off Geant4Calorimeter::Hit, I get a nice folder in the root file with the hits. If DRcaloSiPMHit is based off VHit, will I get this? Or I will have to write something similar to what is done for the Geant4HitData to get this (in which case it is propbably faster for me to evolve it to inherit off Geant4Calorimeter::Hit)

MarkusFrankATcernch commented 3 years ago

You will get all the data member fields of the hit class you produce. Like before you have to generate a dictionary for the new class. You can also to inherit from Geant4Calorimeter::Hit.

SanghyunKo commented 3 years ago

Hi all, I was trying to follow up on the topic (I have to admit that I'm not familiar with exploiting DDSim or DDG4), and it seems that the dual-readout application has to be modified for this case to use the DDSim steering file.

The current dual-readout application only uses DD4hep as a geometry constructor, and the sensitive detector part is based on the Geant4 - like what FCCSW does at the moment if I understand correctly.

Let me take a look and see how I can support the DDSim steering file.

saraheno commented 3 years ago

Hi SangHyuan, I think I can get it to work. I'm using the bulk of your code, and think I only need to change this one part. I think I know how to do it. Let me try it before you trouble yourself...

saraheno commented 3 years ago

okay, I have something usable (although not as detailed as Sang-Hyan's hit code) working, but now I am struggling with the CMakeFile to get the dictionary in so I can see the hits in the root browser. I really don't understand CMake. I attach the CMAKEFile. the error I get is:

-- |> Building DDDualCrystal CMake Error at examples/DDDualCrystal/CMakeLists.txt:86: Parse error. Expected a command name, got right paren with text ")".

-- Configuring incomplete, errors occurred! See also "/home/eno/dd4hep/DD4hep/build/CMakeFiles/CMakeOutput.log". See also "/home/eno/dd4hep/DD4hep/build/CMakeFiles/CMakeError.log".

any help would be appreciated

CMakeLists.txt

andresailer commented 3 years ago

You have a closing parenthesis in line 86. That parenthesis is superfluous and can be removed?

saraheno commented 3 years ago

thanks. I guess there is still something wrong in the cmake file, as it is still not finding the class definition

dictprobl

andresailer commented 3 years ago

Did you push all your code to github?

saraheno commented 3 years ago

yes

andresailer commented 3 years ago

you need to update your namespace to ddDRcalo

https://github.com/saraheno/DD4hep/blob/e133a903131aa959ff8aa40dfa042b565fa2ba60/examples/DDDualCrystal/src/DRcaloSiPMHit.h#L77-L79

saraheno commented 3 years ago

Thanks. My students should have fun this summer playing with these root tuples!

andresailer commented 3 years ago

Great!