Extend onnxruntime gpu interface to producers using onnxruntime #39402
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,7 @@ | ||
| <use name="onnxruntime"/> | ||
| <use name="FWCore/Utilities"/> | ||
| <use name="HeterogeneousCore/CUDAServices" source_only="1"/> | ||
| <use name="FWCore/ServiceRegistry" source_only="1"/> | ||
| <export> | ||
| <lib name="1"/> | ||
| </export> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| #ifndef PHYSICSTOOLS_ONNXRUNTIME_ONNXSESSIONOPTIONS_H | ||
| #define PHYSICSTOOLS_ONNXRUNTIME_ONNXSESSIONOPTIONS_H | ||
|
|
||
| #include "HeterogeneousCore/CUDAServices/interface/CUDAService.h" | ||
| #include "FWCore/ServiceRegistry/interface/Service.h" | ||
| #include "ONNXRuntime.h" | ||
davidlange6 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| #include "onnxruntime/core/session/onnxruntime_cxx_api.h" | ||
| #include <string> | ||
|
|
||
| namespace cms::Ort { | ||
|
|
||
| // param_backend | ||
| // cpu -> Use CPU backend | ||
| // cuda -> Use cuda backend | ||
| // default -> Use best available | ||
| inline ::Ort::SessionOptions getSessionOptions(const std::string ¶m_backend) { | ||
| auto backend = cms::Ort::Backend::cpu; | ||
| if (param_backend == "cuda") | ||
| backend = cms::Ort::Backend::cuda; | ||
|
There was a problem hiding this comment. I'm wondering what should happen if If we want ONNX modules to also avoid using GPUs when (and in the longer term we should think of how to improve the mechanism) There was a problem hiding this comment.
ah - great - I saw the issue at some point, but missed that it was fixed (as the issue was open...) There was a problem hiding this comment. @davidlange6 Here is the code snipped I mentioned in order to make the " if (param_backend == "cuda") {
edm::Service<CUDAService> cs;
if (cs.isAvailable() and cs->enabled()) {
backend = cms::Ort::Backend::cuda;
} else {
edm::Exception ex(edm::errors::UnavailableAccelerator);
ex << "cuda backend requested, but no NVIDIA GPU available in the job";
ex.addContext("Calling cms::Ort::getSessionOptions()");
throw ex;
}
}(although I have a feeling there is room for future simplification in conjunction with the There was a problem hiding this comment. How should we proceed here? Improve the logic now, or merge this PR and make a quick follow-up? There was a problem hiding this comment. (I thought I had written this comment already earlier, but apparently not) We could also make the code here less dependent on the availability of CUDA runtime in the CMSSW build by using if (param_backend == "cuda") {
edm::Service<edm::ResourceInformation> ri;
if (not ri.nvidiaDriverVersion.empty()) {
backend = cms::Ort::Backend::cuda;
} else {
edm::Exception ex(edm::errors::UnavailableAccelerator);
ex << "cuda backend requested, but no NVIDIA GPU available in the job";
ex.addContext("Calling cms::Ort::getSessionOptions()");
throw ex;
}
}
if (param_backend == "default") {
edm::Service<edm::ResourceInformation> ri;
if (not ri->nvidiaDriverVersion().empty()) {
backend = cms::Ort::Backend::cuda;
}
}(although we should probably think of better API in |
||
|
|
||
| if (param_backend == "default") { | ||
| edm::Service<CUDAService> cs; | ||
| if (cs.isAvailable() and cs->enabled()) { | ||
| backend = cms::Ort::Backend::cuda; | ||
| } | ||
| } | ||
|
|
||
| return ONNXRuntime::defaultSessionOptions(backend); | ||
| } | ||
| } // namespace cms::Ort | ||
|
|
||
| #endif | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think both of these should be without the
source_onlyas the shared libraries are needed.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps my understanding of what source_only means here is wrong.. I used this to mean that only the source (eg, the interface) of PhysicsTools/ONNXRuntime needs these packages (eg, I was trying to avoid adding a library dependency that wasn't already there) - do I have that backwards? (@smuzaffar)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
source_onlymeans the package only depend on the headers of its dependent package and there is no linking required. This flag was not need as headers of dependent packages were always available to include but we added this for cxxmodules IBs so that scram can properly build the dependent package module first. And no, there is no backward of this supported.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok - thanks - I've updated accordingly..