Add nTypes to enum

This guaranties that the rest of the code remains consistent if we add additional entries to the enum

/cc @goetz

added ACTS bugfix labels

This merge request affects 4 packages:

• Event/xAOD/xAODMeasurementBase
• Tracking/Acts/ActsEvent
• Tracking/Acts/ActsTrackReconstruction
• Tracking/Acts/ActsTruth

Affected files list will not be printed in this case

Adding @tbold ,@adye ,@goetz ,@jojungge ,@toyamaza ,@cvarni ,@pagessin ,@akraszna as watchers

added EDM Tracking analysis-review-required main review-pending-level-1 labels

CI Result FAILURE (hash 137b069d)

	Athena	AthSimulation	AthGeneration	AnalysisBase	AthAnalysis
externals
cmake
make
tests

Full details available on this CI monitor view. Check the JIRA CI status board for known problems
Athena: number of compilation errors 0, warnings 0
AthSimulation: number of compilation errors 0, warnings 0
AthGeneration: number of compilation errors 0, warnings 0
AnalysisBase: number of compilation errors 0, warnings 0
AthAnalysis: number of compilation errors 0, warnings 0
For experts only: Jenkins output [CI-MERGE-REQUEST-EL9 7949] (remote access info)

Should TrackingSurfaceHelper::s_NMeasTypes be replaced everywhere with xAOD::UncalibMeasType::NSIZE? Certainly, NSIZE seems to be a much better place to define this.

This is an excellent idea, but is xAOD::UncalibMeasType::NSIZE the best name for this? If NSIZE is conventional for Athena enums, then it's good. Otherwise, maybe xAOD::UncalibMeasType::nTypes or xAOD::UncalibMeasType::nUncalibMeasTypes would better describe it.

I was looking for this at some point in the past. There seems to be no standard name for the last "enum". Often is is coined as "Invalid", here would be InvalidMeasType presumably. +1 for meaningful name.

Hi @adye and @tbold ,

I prefer indeed something like nTypes or nUncalibMeasTypes. I can apply the change.

Concerning TrackingSurfaceHelper::s_NMeasTypes I'm not sure. Using there the real size of the enum would inlude entries for muon EDMs that we are not really planning of using there. Right?

s_NMeasTypes is currently set to 4, which includes MdtDriftCircleType. Maybe it was meant to include HGTDClusterType. Has that moved?

I suspect we wouldn't lose much if we allowed all types, but if we want to restrict it to ITk only then I guess we should reduce it to 3. In that case, could we define an extra xAOD::UncalibMeasType enum like nITkTypes?

Change label to review-user-action-required while discussion is taking place. Clara (L1)

added review-user-action-required label and removed review-pending-level-1 label

marked this merge request as draft

added 1 commit

• c118a77f - From NSIZE to nTypes

Compare with previous version

added 1 commit

• 0f6e259d - changes

Compare with previous version

marked this merge request as ready

Jenkins please retry a build

@adye @tbold I have gone for nTypes and adapted the changes accordingly. Concerning TrackingSurfaceHelper::s_NMeasTypes the HGTD has always been at the end of the list, since that is the last addition to the EDMs. I suppose we can address that in a follow up MR if we deem it necessary

added 1 commit

• d7700f8f - Update TrackFindingValidationAlg.h

Compare with previous version

Can you update the MR title?

This merge request affects 4 packages:

• Event/xAOD/xAODMeasurementBase
• Tracking/Acts/ActsEvent
• Tracking/Acts/ActsTrackReconstruction
• Tracking/Acts/ActsTruth

Affected files list will not be printed in this case

Adding @akraszna ,@pagessin ,@jojungge ,@cvarni ,@goetz ,@adye ,@toyamaza ,@tbold as watchers

added review-pending-level-1 label and removed review-user-action-required label

This merge request affects 4 packages:

• Event/xAOD/xAODMeasurementBase
• Tracking/Acts/ActsEvent
• Tracking/Acts/ActsTrackReconstruction
• Tracking/Acts/ActsTruth

Affected files list will not be printed in this case

Adding @cvarni ,@akraszna ,@pagessin ,@adye ,@toyamaza ,@goetz ,@tbold ,@jojungge as watchers

changed title from Add NSIZE to enum to Add nTypes to enum

CI Result FAILURE (hash d7700f8f)

	Athena	AthSimulation	AthGeneration	AnalysisBase	AthAnalysis
externals
cmake
make
tests

Full details available on this CI monitor view. Check the JIRA CI status board for known problems
Athena: number of compilation errors 0, warnings 0
AthSimulation: number of compilation errors 0, warnings 0
AthGeneration: number of compilation errors 0, warnings 0
AnalysisBase: number of compilation errors 0, warnings 0
AthAnalysis: number of compilation errors 0, warnings 0
For experts only: Jenkins output [CI-MERGE-REQUEST-EL9 8022] (remote access info)

CI Result FAILURE (hash 0f6e259d)

	Athena	AthSimulation	AthGeneration	AnalysisBase	AthAnalysis
externals
cmake
make
tests

Full details available on this CI monitor view. Check the JIRA CI status board for known problems
Athena: number of compilation errors 0, warnings 0
AthSimulation: number of compilation errors 0, warnings 0
AthGeneration: number of compilation errors 0, warnings 0
AnalysisBase: number of compilation errors 0, warnings 0
AthAnalysis: number of compilation errors 0, warnings 0
For experts only: Jenkins output [CI-MERGE-REQUEST-EL9 8021] (remote access info)

added 2 commits

• 156b56df - correct size
• 2193cf56 - Merge remote-tracking branch 'origin/AddSizeToEnum' into AddSizeToEnum

Compare with previous version

This merge request affects 4 packages:

• Event/xAOD/xAODMeasurementBase
• Tracking/Acts/ActsEvent
• Tracking/Acts/ActsTrackReconstruction
• Tracking/Acts/ActsTruth

Affected files list will not be printed in this case

Adding @toyamaza ,@goetz ,@cvarni ,@jojungge ,@akraszna ,@adye ,@pagessin ,@tbold as watchers

CI Result SUCCESS (hash 2193cf56)

	Athena	AthSimulation	AthGeneration	AnalysisBase	AthAnalysis
externals
cmake
make
tests

Full details available on this CI monitor view. Check the JIRA CI status board for known problems
Athena: number of compilation errors 0, warnings 0
AthSimulation: number of compilation errors 0, warnings 0
AthGeneration: number of compilation errors 0, warnings 0
AnalysisBase: number of compilation errors 0, warnings 0
AthAnalysis: number of compilation errors 0, warnings 0
For experts only: Jenkins output [CI-MERGE-REQUEST-EL9 8050] (remote access info)

This seems to happen upstream of user analysis code so approving from the analysis side. Tong (AR)

removed analysis-review-required label

added analysis-review-approved label

Changes look fine and the CI is good. Approving. Clara (L1)

added review-approved label and removed review-pending-level-1 label

merged

mentioned in commit ec629ddc