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

Add data extension class for FCCee Drift Chamber #1253

Merged
merged 5 commits into from
May 13, 2024

Conversation

atolosadelgado
Copy link
Contributor

@atolosadelgado atolosadelgado commented Apr 16, 2024

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
Copy link
Contributor

@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.

Copy link

github-actions bot commented Apr 16, 2024

Test Results

   14 files     14 suites   8h 15m 5s ⏱️
  363 tests   363 ✅ 0 💤 0 ❌
2 496 runs  2 496 ✅ 0 💤 0 ❌

Results for commit 3a2f5c2.

♻️ This comment has been updated with latest results.

@atolosadelgado
Copy link
Contributor Author

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
Copy link
Contributor Author

thanks @andresailer for the comments!

@atolosadelgado atolosadelgado marked this pull request as draft May 6, 2024 11:28
@atolosadelgado
Copy link
Contributor Author

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

@atolosadelgado atolosadelgado marked this pull request as ready for review May 13, 2024 11:32
@atolosadelgado
Copy link
Contributor Author

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 :)

@andresailer andresailer enabled auto-merge (rebase) May 13, 2024 11:39
@andresailer andresailer merged commit 372f09b into AIDASoft:master May 13, 2024
13 checks passed
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