Changes are needed to run pixel...

Changes are needed to run pixel digitization with radiation damage without crashes, getting correct hits in long columns, and running faster. See ATLPHYSVAL-654 for discussion.

added Digitization master labels

This merge request affects 2 packages:

• InnerDetector/InDetConditions/PixelConditionsAlgorithms
• InnerDetector/InDetDigitization/PixelDigitization

Affected files list will not be printed in this case

Adding @goetz ,@jchapman ,@stsuno ,@lshan ,@battagl ,@sroe as watchers

added InnerDetector review-pending-level-1 labels

Tagging also @mbomben and @mociduki since they are unfortunately not automatically added as watchers when the radiation damage code is modified. Also tagging @kesedlac for the record.

CI Result FAILURE (hash 19527a91)

	Athena	AthSimulation	AthGeneration	AnalysisBase	AthAnalysis
externals
cmake
make
required tests
optional tests

Full details available on this CI monitor view
Athena: number of compilation errors 1, warnings 1
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-CC7 25670]

Unresolved threads. (L1)

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

Would it be possible to try a rebuild ?

I don't have any compilation error in my local tests, and I cannot find a logfile from the build of the CI test above (I think it's too old, and I missed the notification)

cheers, Tommaso

Jenkins please retry a build

This merge request affects 2 packages:

• InnerDetector/InDetConditions/PixelConditionsAlgorithms
• InnerDetector/InDetDigitization/PixelDigitization

Affected files list will not be printed in this case

Adding @goetz ,@jchapman ,@stsuno ,@lshan ,@battagl ,@sroe as watchers

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

CI Result SUCCESS (hash 19527a91)

	Athena	AthSimulation	AthGeneration	AnalysisBase	AthAnalysis
externals
cmake
make
required tests
optional tests

Full details available on this CI monitor view
Athena: number of compilation errors 0, warnings 1
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-CC7 25793]

Hi @tlari, the compilation warnings are: /var/lib/jenkins/workspace/CI-MERGE-REQUEST-CC7/master/InnerDetector/InDetDigitization/PixelDigitization/src/SensorSimPlanarTool.cxx:240:12: warning: variable 'pixelEta_i' set but not used [-Wunused-but-set-variable] double pixelEta_i = 0.0; ^~~~~~~~~~ /var/lib/jenkins/workspace/CI-MERGE-REQUEST-CC7/master/InnerDetector/InDetDigitization/PixelDigitization/src/SensorSimPlanarTool.cxx:241:12: warning: variable 'pixelPhi_i' set but not used [-Wunused-but-set-variable] double pixelPhi_i = 0.0;

Please take a look at them and also resolve discussions.

Best, Magdalena (L1)

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

added 1 commit

• 2e4f5918 - fix a compilation warning

Compare with previous version

This merge request affects 2 packages:

• InnerDetector/InDetConditions/PixelConditionsAlgorithms
• InnerDetector/InDetDigitization/PixelDigitization

Affected files list will not be printed in this case

Adding @goetz ,@jchapman ,@stsuno ,@lshan ,@battagl ,@sroe as watchers

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

Hi Magda thanks, I have fixed the warning

As for the discussion threads, I think @stsuno and @bnachman should comment

cheers, Tommaso

CI Result SUCCESS (hash 2e4f5918)

	Athena	AthSimulation	AthGeneration	AnalysisBase	AthAnalysis
externals
cmake
make
required tests
optional tests

Full details available on this CI monitor view
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-CC7 25813]

Unresolved threads. Please review the comments and resolve the threads. (L1)

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

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

Thanks, @bnachman and @zmarshal for resolving the threads. Marking for L1 review.

Hi @tlari

I have only two minor comments, plus there is still one unresolved discussion above, therefore reverting to review-user-action-required

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

added 1 commit

• 189327a7 - address comments in merge request

Compare with previous version

This merge request affects 2 packages:

• InnerDetector/InDetConditions/PixelConditionsAlgorithms
• InnerDetector/InDetDigitization/PixelDigitization

Affected files list will not be printed in this case

Adding @goetz ,@jchapman ,@stsuno ,@lshan ,@battagl ,@sroe as watchers

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

CI Result FAILURE (hash 189327a7)

	Athena	AthSimulation	AthGeneration	AnalysisBase	AthAnalysis
externals
cmake
make
required tests
optional tests

Full details available on this CI monitor view
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-CC7 25934]

Jenkins please retry a build

This merge request affects 2 packages:

• InnerDetector/InDetConditions/PixelConditionsAlgorithms
• InnerDetector/InDetDigitization/PixelDigitization

Affected files list will not be printed in this case

Adding @goetz ,@jchapman ,@stsuno ,@lshan ,@battagl ,@sroe as watchers

CI Result SUCCESS (hash 189327a7)

	Athena	AthSimulation	AthGeneration	AnalysisBase	AthAnalysis
externals
cmake
make
required tests
optional tests

Full details available on this CI monitor view
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-CC7 25979]

Nice!

• There is one outstanding conversation that looks like it requires some action
• Some minor coding readability requests

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

added 1 commit

• 38444559 - implementing some comments from MR reviews

Compare with previous version

This merge request affects 2 packages:

• InnerDetector/InDetConditions/PixelConditionsAlgorithms
• InnerDetector/InDetDigitization/PixelDigitization

Affected files list will not be printed in this case

Adding @goetz ,@jchapman ,@stsuno ,@lshan ,@battagl ,@sroe as watchers

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

CI Result SUCCESS (hash 38444559)

	Athena	AthSimulation	AthGeneration	AnalysisBase	AthAnalysis
externals
cmake
make
required tests
optional tests

Full details available on this CI monitor view
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-CC7 26005]

This can be approved once the final threads are resolved so if @tlari can do that we can proceed. Tom - L1 shifter

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

resolved all threads

added review-approved label and removed review-user-action-required label