eic / epic

Geometry Description of the ePIC Experiment
https://eic.github.io/epic
GNU Lesser General Public License v3.0
25 stars 45 forks source link

Insert granularity #771

Closed sebouh137 closed 3 months ago

sebouh137 commented 3 months ago

multisegmentation for the insert

Briefly, what does this PR introduce?

Updates the segmentation for the insert so that it has 3 different segmentations: ~12cm2 in layers 1-16 ~21 cm2 in layers 17-60 for the left side ~25 cm2 in layers 17-60 for the right side

What kind of change does this PR introduce?

Please check if this PR fulfills the following:

Does this PR introduce breaking changes? What changes might users need to make to their code?

no

Does this PR change default behavior?

yes. different cell sizes in the calorimeter insert. Also a few minor changes: removed thinner front layer from the insert. Added gap between left and right sides. Adjusted the width and heights of the insert slightly.

sebouh137 commented 3 months ago

Edited some documentation and requested clarification in some parts. Overall it looks good, assuming that the segmentation you introduced behaves as expected like the plots you've shown before.

thank!

sebouh137 commented 3 months ago

@rymilton Thanks for the feedback. If you are satisfied with my changes and responses, please approve. If you request futher changes, please let me know. Thanks

rymilton commented 3 months ago

Thanks these changes look good. One more thing is that with your branch, I'm seeing this error when using EICrecon: Error in <TGeoVoxelFinder::SortAll>: Volume EcalEndcapPInsert: Cannot make slices on any axis, but I don't see it when using the main branch of epic. Can you reproduce this, and do you know what's causing this?

sebouh137 commented 3 months ago

Thanks these changes look good. One more thing is that with your branch, I'm seeing this error when using EICrecon: Error in <TGeoVoxelFinder::SortAll>: Volume EcalEndcapPInsert: Cannot make slices on any axis, but I don't see it when using the main branch of epic. Can you reproduce this, and do you know what's causing this?

I was getting the same errors, and I found that the fix for it is adding "side" to the fields in the CalorimeterHitsMerger_factory in FHCAL.cc . I am fixing this in https://github.com/eic/EICrecon/pull/1595

    app->Add(new JOmniFactoryGeneratorT<CalorimeterHitsMerger_factory>(
      "HcalEndcapPInsertMergedHits", {"HcalEndcapPInsertRecHits"}, {"HcalEndcapPInsertMergedHits"},
      {
        .readout = "HcalEndcapPInsertHits",
        .fields = {"layer", "slice", "side"},
        .refs = {1, 0},
      },
      app   // TODO: Remove me once fixed
    ));
rymilton commented 3 months ago

Alright, sounds good. I'll approve these changes then.