openvino icon indicating copy to clipboard operation
openvino copied to clipboard

[C API] Refine OV 2.0 C APIs for better expansibility and compatibility

Open riverlijunjie opened this issue 2 years ago • 6 comments

Details:

  • OV C 2.0 definition need better expansibility to make it no changes in future.
    • Change method semantic to strict align with C++ API
    • Strict align C and C++ methods naming
    • Refine partial shape implement to avoid string input
    • Refine property implement
  • Put legacy C API and 2.0 C API into a single library rather than 2 separated libraries.
  • Memory leak handle if exception occurs in memory allocation
  • Remove legacy C API samples completely, replaced by OV 2.0 C APIs samples.
  • Split header file into multiple small header files, also for source code file.

Tickets:

  • 87891

riverlijunjie avatar Jul 17 '22 13:07 riverlijunjie

@ilyachur about put node and model interface into a header file, I agree with you, but one point we need notice, we cannot wrap 1:1 C++ method into C APIs, it is hard and unnecessary.

riverlijunjie avatar Jul 24 '22 11:07 riverlijunjie

@ilyachur about put node and model interface into a header file, I agree with you, but one point we need notice, we cannot wrap 1:1 C++ method into C APIs, it is hard and unnecessary.

I fully understand that in C API cannot be 1-1 aligned with C++. In comments I only would like to provide the idea that one file (maybe except something common.h) should define one thing. At the current moment some header files have a mix of different structures and functions.

ilyachur avatar Jul 24 '22 12:07 ilyachur

@ilyachur about put node and model interface into a header file, I agree with you, but one point we need notice, we cannot wrap 1:1 C++ method into C APIs, it is hard and unnecessary.

I fully understand that in C API cannot be 1-1 aligned with C++. In comments I only would like to provide the idea that one file (maybe except something common.h) should define one thing. At the current moment some header files have a mix of different structures and functions.

Ok, I got your point, will split one thing into one header file.

riverlijunjie avatar Jul 24 '22 14:07 riverlijunjie

To summarise the solution about headers location:

  • Please use one path openvino/c/xxx.h inside the openvino and samples application. We shouldn't have different usage
  • Please use absolute path in our headers '#include "openvino/c/xxx.h"`

ilyachur avatar Aug 09 '22 10:08 ilyachur

As sync with @ilyachur this morning, this PR still need solve most-critical issue: shape definition, default create core, reshape rename and something like that. Other comments will be put into the following PRs. I will submit the most critical issue solution this week.

riverlijunjie avatar Aug 11 '22 06:08 riverlijunjie

Merge this PR as base first. Will continue refine OV 2.0 C API in following PRs.

@wangleis We cannot merge this PR as is.

ilyachur avatar Aug 11 '22 09:08 ilyachur

@riverlijunjie Please fix CI.

ov_capi_test: error while loading shared libraries: libgna.so.2

Why do you have this dependency?

ilyachur avatar Aug 12 '22 02:08 ilyachur

@riverlijunjie Please fix CI.

ov_capi_test: error while loading shared libraries: libgna.so.2

Why do you have this dependency?

Because of static build

ilya-lavrenov avatar Aug 13 '22 11:08 ilya-lavrenov

/azp run openvino-win

ilya-lavrenov avatar Aug 14 '22 12:08 ilya-lavrenov

Azure Pipelines successfully started running 1 pipeline(s).

azure-pipelines[bot] avatar Aug 14 '22 12:08 azure-pipelines[bot]

@riverlijunjie @wangleis Additional requests to this PR:

  • [ ] Unicode support [P2]
  • [x] Improve partial shape and dimension [P0]
  • [x] Improve shape [P0]
  • [x] Align API of C InferRequest with C++ [P1]
  • [ ] Extend Model API (ov_model_output_by_index) [P1]
  • [ ] Update ov::Model documentation [P2]
  • [ ] Update properties API [P0]

@ilyachur got it, let's submit small PRs to close these items one by one from p0 to p2.

riverlijunjie avatar Aug 15 '22 11:08 riverlijunjie