-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/onnxruntime recipe #5180
Conversation
This comment has been minimized.
This comment has been minimized.
d26731d
to
6551952
Compare
This comment has been minimized.
This comment has been minimized.
6551952
to
46e19d2
Compare
This comment has been minimized.
This comment has been minimized.
46e19d2
to
f3cc532
Compare
This comment has been minimized.
This comment has been minimized.
f3cc532
to
122ceaf
Compare
"1.7.1": | ||
sha256: e24ff9a2a21dab74e6dfa821e9a267baa10e9423ffae5f5e404e3a2949eb41e2 | ||
url: https://github.com/microsoft/onnxruntime/archive/v1.7.1.tar.gz | ||
"1.5.3": | ||
sha256: 40fc79f8a3126caba5fcdced260439c7b18c72214fb8f7268d87fac22098dfba | ||
url: https://github.com/microsoft/onnxruntime/archive/v1.5.3.tar.gz | ||
"1.2.0": | ||
sha256: 1ed2a4303d621682c42ebaf99fb9a8ecdc386035b15925b148aa56944abc38c8 | ||
url: https://github.com/microsoft/onnxruntime/archive/v1.2.0.tar.gz |
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.
only one version for the first iteration please, to have quick results from CI
tools.download( | ||
"https://github.com/microsoft/onnxruntime/raw" | ||
"/master/csharp/testdata/squeezenet.onnx", | ||
"squeezenet.onnx" | ||
) |
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.
Not reproducible, downloads outside of source()
should be avoided.
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() doesn't work for test_package
for some reason
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.
@spacelm I think, we can create much more simple .onnx graph about ~10kb, which can be stored in the test folder in the repo, i.e. like it is done in some packages for locale checking etc
This comment has been minimized.
This comment has been minimized.
@SpaceIm thanks for the review, I agree with all feedback will work to fix it |
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
@CAMOBAP Are you still making a recipe? |
@DenisYarullin yep this is still in my TODO list. Unfortunatelly I switched to other work(( I plan to update this PR this week |
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
I have some progress but it's still not enough to push any update, I will push ASAP once will have something working (I hope this week) |
This comment has been minimized.
This comment has been minimized.
122ceaf
to
2dc3183
Compare
@fdgStilla thanks a lot for help with this, once #6085 will be pushed I will continue work on this one BTW which version on OnnxRuntime have you tested with changes in #5180 (comment) it looks > 1.7.1 |
This comment has been minimized.
This comment has been minimized.
@CAMOBAP I did not change the version so it retrieves https://github.com/microsoft/onnxruntime/archive/v1.7.1.tar.gz as you initially wrote it. |
I see that you have |
self._add_definition("USE_AVX2", False) | ||
self._add_definition("USE_AVX512", False) | ||
|
||
self._add_definition("USE_OPENMP", False) |
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.
shouldn't with_openmp
be used here or anywhere else?
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.
it should, but before I need to be able pass tests at least for basic setup...
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@fdgStilla thanks a lot for help, I'm testing all those changes locally rigth now. For some reason conan cannot resolve 'new' receipts line |
3fbfec3
to
2f2a357
Compare
Failure in build 13 (
Note: To save resources, CI tries to finish as soon as an error is found. For this reason you might find that not all the references have been launched or not all the configurations for a given reference. Also, take into account that we cannot guarantee the order of execution as it depends on CI workload and workers availability. |
@fdgStilla i have applied almost all your suggestions thanks again. For now there is two major issues:
If you have any insights on those please let me know |
@CAMOBAP I forgot I was not using your test_package, I used a very simplified test_package like this: cmake_minimum_required(VERSION 3.1)
project(PackageTest CXX)
include(${CMAKE_BINARY_DIR}/conanbuildinfo.cmake)
conan_basic_setup()
set(CMAKE_MODULE_PATH ${CMAKE_BINARY_DIR})
find_package(OnnxRuntime REQUIRED)
add_executable(onnxruntime_test onnxruntime_test.cpp)
target_link_libraries(onnxruntime_test OnnxRuntime::OnnxRuntime) #include <onnxruntime_cxx_api.h>
#include <iostream>
int main()
{
Ort::SessionOptions m_session_options;
std::cout << "Success !" << std::endl;
return 0;
} I don't know why your test is failing, but maybe you can compare with mine and understand the issue. |
@fdgStilla I have created a test program based on https://github.com/microsoft/onnxruntime/blob/master/csharp/test/Microsoft.ML.OnnxRuntime.EndToEndTests.Capi/CXX_Api_Sample.cpp To me
Doesn't looks enough for such complex software as Onnxruntime (but maybe I'm wrong) Looks I need to dig into the test program... |
I agree, I was juste a little lazy... |
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
This pull request has been automatically closed because it has not had recent activity. Thank you for your contributions. |
New recipe for OnnxRuntime/1.7.1
per #4806
conan-center hook activated.