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

DCH v2 digitizer, which smears the position and adds cluster information #27

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

Conversation

atolosadelgado
Copy link

BEGINRELEASENOTES

  • New digitizer for DCH v2 (based on twisted tubes). It calculates cluster information and smears the position along and perpendicular the wire separately. New custom EDM4hep data extension was added to include these data.

ENDRELEASENOTES

Hi,

Please find attached the drift chamber (DCH) digitizer, based on Walaa's code for the cluster number and size calculations. In addition, I added the position smearing based on analytical calculations (I was stuck here, but thanks to Andre we found the problem).

The algorithm works as expected, but I can imagine there is room for improvement regarding naming, coding style in general, so please let me know if I can improve it somehow.

I have been working on a stand alone repository, in case you want to try out: https://github.com/atolosadelgado/DCH_detector

Alvaro.

@BrieucF
Copy link
Collaborator

BrieucF commented Aug 7, 2024

Hi Alvaro, excellent! Can you add a test and add a link between sim and digi hits? See e.g. https://github.com/key4hep/k4RecTracker/pull/28/files

@atolosadelgado
Copy link
Author

atolosadelgado commented Aug 12, 2024

Hi Alvaro, excellent! Can you add a test and add a link between sim and digi hits? See e.g. https://github.com/key4hep/k4RecTracker/pull/28/files

I have added the association and the test. I have inspected the output file and the associations seems to make sense.

Associations are now an additional output collection of the algorithm, but I am not sure if this is the proper way.

You can use the following script to inspect the output file of the test, everything seems consistent

import ROOT
from podio import root_io
import dd4hep as dd4hepModule
from ROOT import dd4hep
from ROOT import TVector3, TFile, TH1D

ofile = TFile("kk.root","update")
hDpw_calc = TH1D("hDpw_calc","",100,0,10)
hDpw_alg  = TH1D("hDpw_alg" ,"",100,0,10)
hDpw_diff = TH1D("hDpw_diff","",100,0,10)

input_file_path = "dch_proton_10GeV_digi.root"
podio_reader = root_io.Reader(input_file_path)

for event in podio_reader.get("events"):
  for dc_hit in event.get("DCH_DigiSimAssociationCollection"):
    digihit =  dc_hit.getDigi()
    simhit  =  dc_hit.getSim()
    digipos = TVector3( digihit.getPosition()[0],digihit.getPosition()[1],digihit.getPosition()[2] )
    simpos  = TVector3(  simhit.getPosition()[0], simhit.getPosition()[1], simhit.getPosition()[2] )
    hit_wire_vector = simpos - digipos
    distance_calculated = hit_wire_vector.Mag()
    distance_from_alg = digihit.getDistanceToWire()
    # print( distance_from_alg - distance_calculated )
    #print( f"{digihit.getEDep()}      {simhit.getEDep()}")
    hDpw_calc.Fill(distance_calculated)
    hDpw_alg.Fill(distance_from_alg)
    hDpw_diff.Fill(distance_calculated-distance_from_alg)
    
ofile.cd()
hDpw_calc.Write()
hDpw_alg.Write()
hDpw_diff.Write()
ofile.Close()

@atolosadelgado
Copy link
Author

@BrieucF the code is ready for review :)

Copy link
Collaborator

@BrieucF BrieucF left a comment

Choose a reason for hiding this comment

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

Thanks Alvaro!

DCHdigi/include/DCHdigi.h Outdated Show resolved Hide resolved
DCHdigi/include/DCHdigi.h Outdated Show resolved Hide resolved
DCHdigi/include/DCHdigi.h Outdated Show resolved Hide resolved
DCHdigi/src/DCHdigi.cpp Outdated Show resolved Hide resolved
DCHdigi/src/DCHdigi.cpp Outdated Show resolved Hide resolved
DCHdigi/include/DCHdigi.h Outdated Show resolved Hide resolved
DCHdigi/include/DCHdigi.h Outdated Show resolved Hide resolved
DCHdigi/test/test_DCHdigi/test_DCHdigi.sh Outdated Show resolved Hide resolved
DCHdigi/test/test_DCHdigi/runDCHdigi.py Outdated Show resolved Hide resolved
DCHdigi/src/DCHdigi.cpp Outdated Show resolved Hide resolved
@BrieucF
Copy link
Collaborator

BrieucF commented Aug 22, 2024

@atolosadelgado
Copy link
Author

Let's add the interface as well: https://github.com/BrieucF/k4RecTracker/blob/fix_for_Dolores/DCHdigi/dataFormatExtension/driftChamberHit.yaml#L66

does this do the trick? how can I test that it works?
47b0d5d

@BrieucF
Copy link
Collaborator

BrieucF commented Aug 28, 2024

Let's add the interface as well: https://github.com/BrieucF/k4RecTracker/blob/fix_for_Dolores/DCHdigi/dataFormatExtension/driftChamberHit.yaml#L66

does this do the trick? how can I test that it works? 47b0d5d

I am surprised that it does not complain that the interfaces edm4hep::TrackerHit: is then defined twice in edm4hep. Let's ask @tmadlener what he thinks. Context: the goal is to avoid having to redefine extension::TrackerHit3D and extension::TrackerHitPlane in order to build an interface, as done here https://github.com/BrieucF/k4RecTracker/blob/fix_for_Dolores/DCHdigi/dataFormatExtension/driftChamberHit.yaml#L66. Last time I tried I could not mix extension objects and edm4hep object in an interface. The latter is indeed very annoying because, in order for subsequent algorithm to work, we have to cast e.g. edm4hep::TrackerHit3D coming from the SIM step into extension::TrackerHit3D and store this duplicated collection in the output rootfile.

std::pair<uint32_t,uint32_t> return_values = {0,0};
uint32_t & Ncl = return_values.first;
uint32_t & ClSz = return_values.second;
//_________________SET NECESSARY PARAMETERS FOR THE CLS ALGORITHM-----WALAA_________________//
Copy link
Collaborator

Choose a reason for hiding this comment

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

All these parameters will be set at every call of the function, since many are hard coded, would'nt it be better to define them in the header once and for all?

DCHdigi/src/DCHdigi.cpp Outdated Show resolved Hide resolved
@atolosadelgado
Copy link
Author

Let's add the interface as well: https://github.com/BrieucF/k4RecTracker/blob/fix_for_Dolores/DCHdigi/dataFormatExtension/driftChamberHit.yaml#L66

does this do the trick? how can I test that it works? 47b0d5d

I am surprised that it does not complain that the interfaces edm4hep::TrackerHit: is then defined twice in edm4hep. Let's ask @tmadlener what he thinks. Context: the goal is to avoid having to redefine extension::TrackerHit3D and extension::TrackerHitPlane in order to build an interface, as done here https://github.com/BrieucF/k4RecTracker/blob/fix_for_Dolores/DCHdigi/dataFormatExtension/driftChamberHit.yaml#L66. Last time I tried I could not mix extension objects and edm4hep object in an interface. The latter is indeed very annoying because, in order for subsequent algorithm to work, we have to cast e.g. edm4hep::TrackerHit3D coming from the SIM step into extension::TrackerHit3D and store this duplicated collection in the output rootfile.

I changed the data extension as you proposed originally (by duplicating code), so the interface can work as expected.

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.

3 participants