revised version of the HLT InDet GPU demonstrator for AthenaMT, resolves the...

revised version of the HLT InDet GPU demonstrator for AthenaMT, resolves the issues identified in !33485 (closed) and replaces !33485 (closed)

This merge request affects 6 packages:

• Trigger/TrigAccel/TrigAccelEvent
• Trigger/TrigAccel/TrigGpuTest
• Trigger/TrigAccel/TrigInDetAccel/TrigInDetAccelerationService
• Trigger/TrigAccel/TrigInDetAccel/TrigInDetAccelerationTool
• Trigger/TrigAccel/TrigInDetCUDA
• Trigger/TrigAlgorithms/TrigFastTrackFinder

Adding @sutt ,@nagano ,@bernius as watchers

added Trigger master review-pending-level-1 labels

CI Result SUCCESS (hash 3048aed3)

	Athena	AthSimulation	AnalysisBase	AthGeneration
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
AnalysisBase: number of compilation errors 0, warnings 0
AthGeneration: number of compilation errors 0, warnings 0
For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 16735]

mentioned in merge request !33485 (closed)

There are quite some changes. Push it to L2 for review.

Xiaozhong (L1)

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

Have no idea - I just stated the requirement for my code to function correctly (see the my remark about capturing the geometry). The BeginRun incident handling works fine.

Also would be possible to see any examples how this start() method is used ?

I have just done a quick test after replacing the BeginRun handle with start(). The results on the MC data I am using are identical because the detector layers average coordinates and boundaries are the same before and after the alignment. But the order of calls is certainly incorrect as the geometry export to the GPU happens before the conditions are applied (e.g. before INFO created CondCont with key 'ConditionStore+/Indet/Align' etc.). So, for this particular case (track seeding) we probably can use start() but for the other parts of the accelerated tracking workflow such as data preparation and track following this will be incorrect.

I am attaching the athena.log file here...athena.log just search for 'TrigInDetAccelerationSvc 0 INFO GPU condition data gathering and export ...'

Thanks, but this means BeginRun will also not work (or it just happens to work in your case). We will need to migrate the geometry extraction then to a conditions algorithm that runs after the corrections have been applied. I suggest we just leave the code with BeginRun for the moment then.

resolved all threads

The code still needs to be check in detail, but I see a big issue that should be addressed first. The code should not have any raw pointers, instead one should use smart pointers (e.g., unique_ptr) or a local variable. Marking for user action to fix raw pointers. (L2)

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

If possible, could you please be more specific - which package, for instance.

added 1 commit

• 2c3e5e58 - resolving conflicts due to the update of TrigFastTrackFinder CMakeLists.txt

Compare with previous version

added 191 commits

• 2c3e5e58...a07d4724 - 190 commits from branch atlas:master
• 91c059e1 - Merge branch 'master' into 'master-revised-mt-id-gpu-demonstrator'

Compare with previous version

This merge request affects 6 packages:

• Trigger/TrigAccel/TrigAccelEvent
• Trigger/TrigAccel/TrigGpuTest
• Trigger/TrigAccel/TrigInDetAccel/TrigInDetAccelerationService
• Trigger/TrigAccel/TrigInDetAccel/TrigInDetAccelerationTool
• Trigger/TrigAccel/TrigInDetCUDA
• Trigger/TrigAlgorithms/TrigFastTrackFinder

Adding @sutt ,@nagano ,@bernius as watchers

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

CI Result FAILURE (hash 91c059e1)

	Athena	AthSimulation	AnalysisBase	AthGeneration
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
AnalysisBase: number of compilation errors 0, warnings 0
AthGeneration: number of compilation errors 0, warnings 0
For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 16869]

Hi,

This is a prototype (i.e. it will only run in GPU tests) so I think the move to smart pointers and stack variables can wait for a subsequent MR.

Cheers, Stewart

The CI failure is known and unrelated.

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

merged

mentioned in commit 8d516e31

added sweep:ignore label

@demelian this is causing ATLASRECTS-5584. You should not be compiling components into a library. In general make sure to never compile the same source file into two libraries. So the file should look like:

atlas_add_library( TrigInDetAccelerationToolLib
                   src/*.cxx
                   ... list of libs... )
atlas_add_component( TrigInDetAccelerationTool
                     src/components/*.cxx
                     TrigInDetAccelerationToolLib )

Thanks Frank. @demelian would you be able to fix this today? If not, someone else might need to make a MR.

Yes, I will fix this problem today and submit a new MR

I have implemented what @fwinkl has suggested and rebuild the TrigInDetAccelerationTool together with TrigFastTrackFinder. The compilation is OK but when I run a test I still see

Py:ConfigurableDb INFO Read module info for 5549 configurables from 5 genConfDb files Py:ConfigurableDb WARNING Found 1 duplicates among the 5 genConfDb files : Py:ConfigurableDb WARNING -------------------------------------------------- Py:ConfigurableDb WARNING -: - [ ] Py:ConfigurableDb WARNING -------------------------------------------------- Py:ConfigurableDb WARNING -TrigInDetAccelerationTool: TrigInDetAccelerationTool.TrigInDetAccelerationToolConf - ['TrigFastTrackFinder.TrigFastTrackFinderConf'] Py:ConfigurableDb WARNING Fix your cmt/requirements file !!

The problem is that the TrigInDetAccelerationTool contains both the abstract interface (needed for TrigFastTrackFinder) and the component AlgTool implementation. I would add another package for abstract interfaces only (TrigInDetAccelerationInterfaces), and move the abstract interfaces of TrigInDetAccelerationTool and TrigInDetAccelerationSvc in the new package. This should simplify both Tool and Svc CMakeLists and also dependencies in the TrigFastTrackFinder.

I don't think that's the problem. The interfaces don't matter here. The warnings is triggere by multiple compilations of the same DECLARE_COMPONENT statement. I think what you are seeing is rather an issue that it still picks it up form the underlying release. So I think your fix should work in a full build.

Yes I agree with @fwinkl. If you make a MR we can fast-track it through.

mentioned in merge request !34771 (merged)

OK, done - please see !34771 (merged)

mentioned in merge request !35131 (closed)