Femcal update #855
Femcal update #855
Conversation
for more information, see https://pre-commit.ci
|
Auto check uses craterlake for overlap checks. Do I change craterlake (and all other craterlake_*)? |
Yes, please update all configurations to use your new xml. |
src/forwardEcalMap.h
Outdated
There was a problem hiding this comment.
Can we move relevant constant definitions to the forwardEcal.xml, and relevant function definitions to forwardEcal_geo.cpp. I see, there are many unused definitions, please get rid of them.
There was a problem hiding this comment.
I would like to avoid that. This piece of code is used elsewhere. So I'd like to keep all those functions and constants, even unused ones for geometry definisions, in one piece. I can think of moving it out from this dir and put it somewhere else where I can include from geometry code, reconstruction code, QA codes, macro to make drawings, etc etc.
There was a problem hiding this comment.
There is no public API to be exported from epic geometry, it's only a DD4hep plugin. We can't maintain codes that have no public use.
Is it possible to split geometry-unrelated stuff into a separate file that you can include both if you prefer to do so in you workflow?
This comment was marked as resolved.
This comment was marked as resolved.
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
|
Googled but no idea how to resolve: |
|
I'm not sure if removal of forward EMcal and Hcal + HCal insert in configurations/craterlake_material_map.yml in |
|
Since this removes forward ecal insert, we need to address benchmarks update eic/EICrecon#1852 (comment) before this can be merged. |
### Briefly, what does this PR introduce? Remove EcalEndcapPInsert since it will not be built. See : eic/epic#855 for removing it from geometry. I'm not sure if I should remove it from : https://github.com/eic/EICrecon/blob/c9ea43d6e21feaafe78bc9f992f3ca70abce51ef/src/services/io/podio/JEventProcessorPODIO.cc#L255 https://github.com/eic/EICrecon/blob/c9ea43d6e21feaafe78bc9f992f3ca70abce51ef/.github/janadot.groups#L7 How can I remove it from auto checks? ### What kind of change does this PR introduce? - [ ] Bug fix (issue #__) - [ ] New feature (issue #__) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [x] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? No more EcalEndcapPInsert ### Does this PR change default behavior? No more EcalEndcapPInsert --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Dmitry Kalinkin <dmitry.kalinkin@gmail.com>
Briefly, what does this PR introduce?
New Forward EM calorimeter geometry with current design. There are 2 models, homogeneous and Wpowder+ScFi.
Also removes active insert, which won't be built.
See following links for some more details:
https://www.star.bnl.gov/~akio/epic/geometry/index.html
https://www.star.bnl.gov/~akio/epic/reco/index.html
https://www.star.bnl.gov/~akio/epic/hadron/index.html
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?
See also PR for EICrecon for digitization change
eic/EICrecon#1848
Does this PR change default behavior?
Not until default configuration files start to use new geometry