Skip to content

Migrate DALI TF plugin to C API 2.0 #5904

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

Merged
merged 3 commits into from
May 5, 2025
Merged

Conversation

mzient
Copy link
Contributor

@mzient mzient commented Apr 30, 2025

Category:

Refactoring (Redesign of existing code that doesn't affect functionality)

Description:

This change abandons the (not really) C API in favor of C API 2.0 in Tensorflow plugin.

Additional information:

This change requires #5898.

Executor memory stats are not implemented in the new C API and the code that prints them is disabled. We can either remove the code or re-implement those in a follow-up.

Affected modules and functionalities:

DALI Tensorflow plugin

Key points relevant for the review:

Tests:

TF tests
TF L3 tests (RN50 and YOLO)

  • Existing tests apply
  • New tests added
    • Python tests
    • GTests
    • Benchmark
    • Other
  • N/A

Checklist

Documentation

  • Existing documentation applies
  • Documentation updated
    • Docstring
    • Doxygen
    • RST
    • Jupyter
    • Other
  • N/A

DALI team only

Requirements

  • Implements new requirements
  • Affects existing requirements
  • N/A

REQ IDs: N/A

JIRA TASK: N/A

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [27754656]: BUILD STARTED

@mzient mzient force-pushed the c_api2_tf_plugin branch from 17a4d10 to adc5473 Compare April 30, 2025 13:27
@dali-automaton
Copy link
Collaborator

CI MESSAGE: [27755682]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [27755682]: BUILD PASSED

Comment on lines 305 to 306
params.exec_type_present = 1;
params.exec_type = exec_type;
Copy link
Collaborator

Choose a reason for hiding this comment

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

DALI_SET_PARAM could be used there

mzient added 2 commits May 5, 2025 12:00
- change stubgen to point to dali.h
- change stuben to use DALI_API instead of DLL_PUBLIC
- rework error handling to use DALI error codes
- use managed handles to avoid resource leaks

Signed-off-by: Michał Zientkiewicz <[email protected]>
Signed-off-by: Michał Zientkiewicz <[email protected]>
@mzient mzient force-pushed the c_api2_tf_plugin branch from 16b8f87 to 9dce358 Compare May 5, 2025 10:02
Signed-off-by: Michał Zientkiewicz <[email protected]>
@dali-automaton
Copy link
Collaborator

CI MESSAGE: [27945405]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [27945405]: BUILD PASSED

@mzient mzient merged commit 0771bde into NVIDIA:main May 5, 2025
6 checks passed
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.

4 participants