Remove device initialization from library #385
Conversation
SDL/EndcapGeometry.cc
Outdated
| } | ||
|
|
||
| void SDL::EndcapGeometry<SDL::Dev>::load(std::string filename) { | ||
| template <typename TQueue> | ||
| void SDL::EndcapGeometry<SDL::Dev>::load(TQueue& queue, std::string filename) { |
There was a problem hiding this comment.
does this need to be a template? can SDL::QueueAcc be used instead?
There was a problem hiding this comment.
Yeah, I was also thinking about this. I think now we don't really need any templates since we could just use SDL::Dev, SDL::DevAcc and SDL::QueueAcc. So I was deciding whether to sticking with templated functions like in most of the code.
SDL/LST.cc
Outdated
| @@ -74,7 +76,8 @@ void SDL::LST<SDL::Acc>::loadAndFillES(alpaka::QueueCpuBlocking& queue, struct m | |||
| pLStoLayer); | |||
| } | |||
|
|
|||
| void SDL::LST<SDL::Acc>::run(SDL::QueueAcc& queue, | |||
| void SDL::LST<SDL::Acc>::run(SDL::Dev& devAccIn, | |||
| SDL::QueueAcc& queue, | |||
There was a problem hiding this comment.
does it make sense to have a queue and a device separately? Isn't a queue uniquely attached to a device?
There was a problem hiding this comment.
That's a good point. I'll tidy things up.
|
This should be working now, but I'll check if I there are things to clean up. /run standalone |
|
The PR was built and ran successfully in standalone mode. Here are some of the comparison plots.
The full set of validation and comparison plots can be found here. |
|
The PR was built and ran successfully with CMSSW. Here are some plots. OOTB All Tracks
The full set of validation and comparison plots can be found here. |
|
@ariostas |
|
This is ready for review. I'll elaborate a bit more regarding the changes. As the PR title suggests, the devices (both host and accelerator) are no longer initialized in the library and are no longer global variables. Instead, accelerator queues are passed as arguments both in standalone and CMSSW. When the host device is needed, it now uses a CMSSW function to get it. Apart from that, I also made some changes so that we use types defined in CMSSW instead of defining our own types. They mostly match, but |
|
/run cmssw 21 I think the last commit in 21 was after the last test was triggered |
|
The PR was built and ran successfully with CMSSW. Here are some plots. OOTB All Tracks
The full set of validation and comparison plots can be found here. |
Currently, a device and queue are initialized within the library as global variables. This can cause various issues.
edmWriteConfigsto crash when compiling theLSTpackage for ROCm since the machines we use don't have any AMD cards.As far as I understand, these two things could result in using a higher (or different) computational resources than what CMSSW assigned for the job, so it can end up being killed.
This PR is still a bit rough, and I haven't worked on the CMSSW side. I'll give a better description of the changes once it's ready.