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

Adding track nholes to EDM #147

Merged
merged 12 commits into from
Jun 6, 2024
Merged

Conversation

madbaron
Copy link
Contributor

@madbaron madbaron commented May 3, 2022

BEGINRELEASENOTES

  • Add Nholes and and subdetectorHoleNumbers to the Track for keeping track of missing hits in a Track.

ENDRELEASENOTES

Inspired by today's discussion at the Joint Tracker and Calorimeter Meeting https://agenda.infn.it/event/31242/,
I have added the number of holes to the LCIO track EDM

@madbaron
Copy link
Contributor Author

madbaron commented May 3, 2022

Mmh, this was meant to go to the MuonColliderSoft repository (sorry I misclicked in the PR creation).
However, this is likely to be useful also upstream, so I leave it here for evaluation.

Cheers,
Federico

Copy link
Contributor

@tmadlener tmadlener left a comment

Choose a reason for hiding this comment

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

From a quick glance, the main missing things are missing version checks in the readers to not break the reading of older files.

src/cpp/src/SIO/SIOTrackHandler.cc Outdated Show resolved Hide resolved
src/cpp/src/SIO/SIOTrackHandler.cc Outdated Show resolved Hide resolved
src/java/hep/lcio/implementation/sio/SIOTrack.java Outdated Show resolved Hide resolved
src/java/hep/lcio/implementation/sio/SIOTrack.java Outdated Show resolved Hide resolved
@madbaron
Copy link
Contributor Author

@tmadlener I have added the version checks. What else do we need to move forward?
Thanks!

Copy link
Contributor

@tmadlener tmadlener left a comment

Choose a reason for hiding this comment

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

This looks to be in quite good shape now. I have only one real comment about the version check (see below). @gaede the I/O parts of the implementation look good to me, but maybe I missed something?

src/cpp/include/IMPL/TrackImpl.h Show resolved Hide resolved
src/python/pyLCIO/io/Reader.py Show resolved Hide resolved
src/cpp/src/SIO/SIOTrackHandler.cc Outdated Show resolved Hide resolved
Copy link
Contributor

@tmadlener tmadlener left a comment

Choose a reason for hiding this comment

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

In order to make CI pass again, we have to bump the version manually here:

LCIO/CMakeLists.txt

Lines 14 to 16 in e8c5032

SET( LCIO_VERSION_MAJOR 2 )
SET( LCIO_VERSION_MINOR 21 )
SET( LCIO_VERSION_PATCH 0 )

I would go to 2.22.99 to sort of indicate that this is a "development" version.

@madbaron
Copy link
Contributor Author

In order to make CI pass again, we have to bump the version manually here:

LCIO/CMakeLists.txt

Lines 14 to 16 in e8c5032

SET( LCIO_VERSION_MAJOR 2 )
SET( LCIO_VERSION_MINOR 21 )
SET( LCIO_VERSION_PATCH 0 )

I would go to 2.22.99 to sort of indicate that this is a "development" version.

done

@tmadlener tmadlener merged commit 0bf2a45 into iLCSoft:master Jun 6, 2024
10 checks passed
@madbaron madbaron deleted the track_holes branch June 6, 2024 14:27
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.

4 participants