Add data extension class for FCCee Drift Chamber

AIDASoft / DD4hep

Detector Description Toolkit for High Energy Physics

http://dd4hep.cern.ch

GNU Lesser General Public License v3.0

47 stars 93 forks source link

Add data extension class for FCCee Drift Chamber #1253

Closed atolosadelgado closed 2 months ago

atolosadelgado commented 3 months ago

Hi,

I went for the option of using a data extension for the drift chamber detector to store the parameters needed to later build the geometry and provide some functionalities as well. It seems these data and functionalities are useful at other stages (digitization and reconstruction).

I have developed the data extension class as part of the geometry within the k4geo repository, link, but I thought it could be useful to upstream it to DD4hep. What do you think?

BEGINRELEASENOTES

DDRec/DCH_info.h: Add data extension class for FCCee Drift Chamber. This class provides data storage and ancillary functionalities needed to build the geometry.

ENDRELEASENOTES

MarkusFrankATcernch commented 3 months ago

@atolosadelgado Hi Alvaro,

given the nature of these changes: it is not necessary that the implementation is so tightly coupled to DDRec. Please put the proposed changes into separate files (inside the DDRec package if you want) and it should be all OK. Any client wanting to benefit then only has to include these headers. Including it in the DetectorData.h/cpp files I think is slightly too intrusive.

github-actions[bot] commented 3 months ago

Test Results

14 files 14 suites 8h 15m 5s :stopwatch: 363 tests 363 :white_check_mark: 0 :zzz: 0 :x: 2 496 runs 2 496 :white_check_mark: 0 :zzz: 0 :x:

Results for commit 3a2f5c26.

:recycle: This comment has been updated with latest results.

atolosadelgado commented 2 months ago

Hi,

I have moved the data extension to a dedicated file. The class is header only. I noticed that for the other data extensions a dictionary is created here, do I have to create the dictionary as well? Why a dictionary is needed?

Thank you for your time :)

atolosadelgado commented 2 months ago

thanks @andresailer for the comments!

atolosadelgado commented 2 months ago

Brieuc wanted me to add another functionality to the class, I hope I can implemented it today, please do not merge it yet :)

atolosadelgado commented 2 months ago

We will keep the functionality where it is needed now. If in the future it is needed in more places we can upstream it here. Please review and merge at your earliest convenience :)