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

[Documentation] Mixing CXX API and C API dtypes #17498

Closed
gedoensmax opened this issue Sep 11, 2023 · 10 comments
Closed

[Documentation] Mixing CXX API and C API dtypes #17498

gedoensmax opened this issue Sep 11, 2023 · 10 comments
Labels
api issues related to all other APIs: C, C++, Python, etc. documentation improvements or additions to documentation; typically submitted using template

Comments

@gedoensmax
Copy link
Contributor

Describe the documentation issue

What would be the nicest way to allocate additional tensors given a session ?

I tried allocating tensors given a session but it feels unnatural as all the CreateTensor APIs need C API structs. Not even entirely sure if it works that way.

const std::unique_ptr<Ort::Session> ort_session 
const std::unique_ptr<Ort::Allocator> allocator = ort_session->getOrtAllocator();

auto ort_value  = Ort::Value::CreateTensor(
reinterpret_cast<OrtAllocator*>(allocator.get()), // This feels bad,
                                                                         shape.data(),
    shape.dimension(),
 type);

Below links show the mixed C and CXX structs usage in the CXX API. It would be great to get an idea on how to best do this.

A way to resolve this would be to call Alloc on the allocator and call CreateTensor with memory info and a raw buffer.

Page / URL

No response

@gedoensmax gedoensmax added the documentation improvements or additions to documentation; typically submitted using template label Sep 11, 2023
@natke natke added the api issues related to all other APIs: C, C++, Python, etc. label Sep 11, 2023
@skottmckay
Copy link
Contributor

const std::unique_ptr<T> doesn't really make sense to me. A unique_ptr has ownership, but 'const' implies it doesn't really.

The C++ API is a convenience wrapper over the C API rather than a separate API. As such it's tightly bound to the C API types.

You can create an allocator with either Ort::AllocatorWithDefaultOptions or Ort::Allocator where you pass in memory info.

Simply pass the ORT C++ type - it will automatically convert to a pointer to the wrapped C API type. In this case an Ort::Allocator instance will convert to OrtAllocator* automatically.

e.g.

    Ort::AllocatorWithDefaultOptions allocator;
    Ort::Session session = Ort::Session(*ort_env, model.c_str(), Ort::SessionOptions{});

    std::vector<int64_t> shape{1, 2, 3, 4};
    auto ort_value = Ort::Value::CreateTensor<float>(allocator, shape.data(), shape.size());

@gedoensmax
Copy link
Contributor Author

This is exactly what I tried with the result being:

message : could be 'Ort::Value Ort::Value::CreateTensor(OrtAllocator *,const int64_t *,size_t,ONNXTensorElementDataType)'
ORTTensor.cpp(59,63): message : 'Ort::Value Ort::Value::CreateTensor(OrtAllocator *,const int64_t *,size_t,ONNXTensorElementDataType)': cannot convert argument 1 from 'const Ort::Allocator *' to 'OrtAllocator *' 

The compile chain is VS build tools 2022.

@yuslepukhin
Copy link
Member

Please, check if your Ort::Allocator argument is const and make it non-const. Allocators are used to allocate memory and they state changes.

@gedoensmax
Copy link
Contributor Author

Oh yeah sorry I noticed this shortly after posting as well and tried without const. Did not change something.

@gedoensmax
Copy link
Contributor Author

@scottmckay just a friendly ping ;) I believe it is just some compiler warning magic, but it would be great to understand this better.

@gedoensmax
Copy link
Contributor Author

@JTischbein I believe you also hit problems with allocators right ?

@skottmckay
Copy link
Contributor

@gedoensmax sorry - didn't notice this as my github id is skottmckay not the 'scottmckay' you tagged.

It seems like your issue is due to you declaring the allocator as being const:
const std::unique_ptr<Ort::Allocator> allocator = ort_session->getOrtAllocator();


I assume there's some helper code or something involved as I don't see a getOrtAllocator in our code.

Again, I'm not sure what a const std::unique_ptr<T> as a local variable is meant to be given a unique_ptr has ownership. When you create a unique_ptr it needs to be non-const. If you're referring to the contents of the unique_ptr elsewhere you could have a const unique_ptr<T>&, but in general you want to hide the usage of the unique_ptr so a const T& to the pointer inside the unique_ptr is preferred.

e.g.

// DoSomething doesn't need to care if the T is in a unique_ptr or not, so we take a const T&
void DoSomething(const T& value) { ... }

unique_ptr<T> instance = std::make_unique<T>(...);  // this owns the T instance and deletes it when it goes out of scope.
DoSomething(*instance);

@gedoensmax
Copy link
Contributor Author

@skottmckay sorry for mistyping you gh id, for some reason it always takes forever to auto complete.
Let me take a step back and give an exact reproducing example, sorry for not doing so in the first place I tried too many combinations and did not extract a good reproducer.
If I ingest the following code in this line, it does not compile.

const auto mem_info = Ort::MemoryInfo("Cuda",
                                      OrtAllocatorType::OrtDeviceAllocator,
                                      0,
                                      OrtMemType::OrtMemTypeDefault);
auto allocator = Ort::Allocator(session_, &mem_info);
std::vector<int64_t> shape{1, 1, 64, 64};
auto ort_value = Ort::Value::CreateTensor(&allocator, shape.data(), shape.size(),
                                          ONNX_TENSOR_ELEMENT_DATA_TYPE_FLOAT);

When adding appropriate, and if I understand even non braking reinterpret_cast's as internally the classes are the same then it works:

const auto mem_info = Ort::MemoryInfo("Cuda",
                                      OrtAllocatorType::OrtDeviceAllocator,
                                      0,
                                      OrtMemType::OrtMemTypeDefault);
auto allocator = Ort::Allocator(session_, reinterpret_cast<const OrtMemoryInfo*>(&mem_info));
std::vector<int64_t> shape{1, 1, 64, 64};
auto ort_value = Ort::Value::CreateTensor(reinterpret_cast<OrtAllocator*>(&allocator), shape.data(), shape.size(),
                                          ONNX_TENSOR_ELEMENT_DATA_TYPE_FLOAT);

Nonetheless this looks quite unintuitive for a C++ API, while the docs show the correct types this is mangles C and C++ API IMO. Or I am still doing something wrong.

@skottmckay
Copy link
Contributor

The C++ API is a lightweight convenience wrapper over the C API (vs. a 'real' parallel standalone C++ API). Due to this, a reference to a C++ API class implicitly converts to a pointer to the C API type, and the arguments to the C++ API methods are the C API types.

So in your original code the only issue is you're passing a pointer to the C++ class instead of letting the implicit conversion to the C API type happen. The fix is to remove the '&' from &mem_info when constructing the allocator, and from the &allocator argument to CreateTensor.

  auto allocator = Ort::Allocator(session_, mem_info);
  // ...
  auto ort_value = Ort::Value::CreateTensor(allocator, shape.data(), shape.size(),
                                            ONNX_TENSOR_ELEMENT_DATA_TYPE_FLOAT);

With that change it compiles for me.

@gedoensmax
Copy link
Contributor Author

Ok thank you a lot for explaining, that works. I did not know that concept behind the C++ API and somehow got around it so far.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api issues related to all other APIs: C, C++, Python, etc. documentation improvements or additions to documentation; typically submitted using template
Projects
None yet
Development

No branches or pull requests

4 participants