Library for ORT execution #13522
Library for ORT execution #13522
Conversation
…at16_t. ort_interface.h now usuable as library (not header!) :party:
|
Ping @davidrohr |
|
REQUEST FOR PRODUCTION RELEASES: This will add The following labels are available |
Common/ML/CMakeLists.txt
Outdated
| o2_add_library(ML | ||
| SOURCES src/ort_interface.cxx | ||
| TARGETVARNAME targetName | ||
| PUBLIC_LINK_LIBRARIES O2::Framework ONNXRuntime::ONNXRuntime) No newline at end of file |
There was a problem hiding this comment.
brauchst du hier PUBLIC, oder geht PRIVATE auch?
Common/ML/include/ML/GPUORTFloat16.h
Outdated
| // or submit itself to any jurisdiction. | ||
|
|
||
| /// \file GPUORTFloat16.h | ||
| /// \author Christian Sonnabend <christian.sonnabend@cern.ch> |
There was a problem hiding this comment.
Hast du das selbst geschrieben, oder von github kopiert?
In letzten Fall musst du den korrekten Autor angeben, und schauen ob der code unter GPL3 lizensiert ist. Jedenfalls haben wir kein copyright dafür.
Ansonten leg ein 3rdparty Verzeichnis wie Framework/Foundation/3rdparty oder GPU/GPUTracking/display/3rdparty an. Das wird dann auch im CodeChecker ausgenommen: https://github.com/alisw/alidist/blob/1916f6d88d42959097998d9481b517dc1c1ea84d/o2checkcode.sh#L71
There was a problem hiding this comment.
Ja, das ist komplett 3rd party von ONNX. Die source geb ich auch im header an, da kopier ich dann vielleicht einfach direkt die file vom ONNX repo und pack die in das Verzeichnis das du oben angegeben hast
Common/ML/include/ML/ort_interface.h
Outdated
| #include <thread> | ||
|
|
||
| // O2 includes | ||
| #include "GPUORTFloat16.h" |
There was a problem hiding this comment.
Wenn du den Datentyp hier nicht benutzt, solltest du den header auch nicht includen.
| target_link_libraries(${mergertargetName} PRIVATE OpenMP::OpenMP_CXX) | ||
| endif() | ||
|
|
||
| o2_add_executable(onnx-interface |
There was a problem hiding this comment.
Wenn es ein test ist, vielleicht o2_add_test?
There was a problem hiding this comment.
Den workflow kann ich für einen PR, der gemerged wird, sowie so weglassen. Das ist nur momentan zum testen und würd ich dann rausnehmen
|
Ideally, we should not have merge commits. Can you rebase without merge commits. Also, you should squash cleanups. |
I won't need the history afterwards. For now I'm just pushing the recent changes into this branch. I will also soon add the code in the actual clusterization code (although I might open a separate PR for that) |
|
This PR did not have any update in the last 30 days. Is it still needed? Unless further action in will be closed in 5 days. |
Please consider the following formatting changes to AliceO2Group#13522
3 participants
Adding a library for the execution of ONNX models with GPU execution provider and float16 datatype (OrtDataType::Float16_t -> a wrapper around a uint16_t)