Skip to content
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

fix issue#35, update ort to v2 #106

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

broccoliSpicy
Copy link

#35

@Ngalstyan4 Ngalstyan4 requested a review from var77 March 22, 2024 17:35
@broccoliSpicy
Copy link
Author

just a draft to see how the CI test results go

@broccoliSpicy
Copy link
Author

broccoliSpicy commented Mar 22, 2024

onnxruntime needs to be update to v1.17.1 as ort v2 requires

@var77
Copy link
Collaborator

var77 commented Mar 22, 2024

Thanks for the contribution @broccoliSpicy , I will do some tests on CUDA to make sure everything works correctly and merge this in next week!

@broccoliSpicy
Copy link
Author

Thanks for the contribution @broccoliSpicy , I will do some tests on CUDA to make sure everything works correctly and merge this in next week!

Thank you so much! @var77
I am actually quick diffident about this PR haha, I don't have access to CUDA or other platforms to test it and I feel I am just fumbling around

@@ -306,7 +309,8 @@ impl EncoderService {

let num_cpus = num_cpus::get();

let encoder = SessionBuilder::new(environment)?
let _ = &ONNX_ENV;
let encoder = Session::builder()?
.with_parallel_execution(true)?
.with_intra_threads(num_cpus as i16)?
.with_optimization_level(GraphOptimizationLevel::Level3)?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just tested with GPU server, the default env seems to not work, but I have put

            .with_execution_providers([
                CUDAExecutionProvider::default().build(),
                OpenVINOExecutionProvider::default().build(),
                CPUExecutionProvider::default().build(),
            ])?

on Session::builder()? and removed the ONNX_ENV completely and it started to use the GPU

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you so much!
feel free to edit it in my branch

@var77
Copy link
Collaborator

var77 commented Apr 18, 2024

Quick update on this: I am waiting ORT to have v2 stable release, make sure that nothing breaks our integration in that release and merge this PR with stable v2 version.

@broccoliSpicy
Copy link
Author

Quick update on this: I am waiting ORT to have v2 stable release, make sure that nothing breaks our integration in that release and merge this PR with stable v2 version.

That makes sense, thank you for your update!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants