-
Notifications
You must be signed in to change notification settings - Fork 656
Fix enum handling. #6150
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 enum handling. #6150
Conversation
Signed-off-by: Michał Zientkiewicz <[email protected]>
Greptile SummaryFixed enum handling in NDD (NVIDIA DALI Dynamic) Tensor and Batch constructors to properly convert DALI enum types when explicit Key Changes:
Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Tensor/Batch
participant Constructor
participant DType
participant numpy
participant Backend
User->>Tensor/Batch: Create with dtype=enum
Tensor/Batch->>Constructor: __init__(data, dtype=enum)
alt dtype is not DType instance
Constructor->>DType: _dtype(enum)
DType-->>Constructor: DType object
end
alt dtype.kind == DType.Kind.enum
Constructor->>numpy: Convert data to int32
numpy-->>Constructor: int32 array
Constructor->>Backend: Create TensorCPU/GPU(int32_array)
Backend-->>Constructor: Storage with int32 dtype
Constructor->>Backend: storage.reinterpret(dtype.type_id)
Backend-->>Constructor: Storage with enum type_id
else dtype is regular type
Constructor->>numpy: Convert data to numpy_type
numpy-->>Constructor: typed array
Constructor->>Backend: Create TensorCPU/GPU(typed_array)
Backend-->>Constructor: Storage with correct dtype
end
Constructor-->>Tensor/Batch: Initialized object
Tensor/Batch-->>User: Tensor/Batch with enum dtype
|
Greptile's behavior is changing!From now on, if a review finishes with no comments, we will not post an additional "statistics" comment to confirm that our review found nothing to comment on. However, you can confirm that we reviewed your changes in the status check section. This feature can be toggled off in your Code Review Settings by deselecting "Create a status check for each PR". |
|
CI MESSAGE: [40970165]: BUILD STARTED |
|
CI MESSAGE: [40970165]: BUILD PASSED |
|
|
||
| self._storage = _backend.TensorCPU( | ||
| np.array(data, dtype=nvidia.dali.types.to_numpy_type(dtype.type_id)), | ||
| np.array(data, dtype=numpy_type), |
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.
| np.array(data, dtype=numpy_type), | |
| np.asarray(data, dtype=numpy_type), |
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.
I think I actually need this copy.
|
CI MESSAGE: [40997933]: BUILD STARTED |
|
CI MESSAGE: [40997933]: BUILD PASSED |
Category:
Bug fix (non-breaking change which fixes an issue)
Other (dependencies)
Description:
This PR fixes the handling of DALI enums in NDD tensors and batches.
Prior to this change, when
dtypewas passed to Tensor constructor, DALI enums were not properly converted and an error was raised. This PR fixes that and adds tests for Tensor and Batch.This PR also adds numpy as an explicit DALI dependency (we require it anyway to perform basic tasks).
Additional information:
Affected modules and functionalities:
Key points relevant for the review:
Tests:
The tests check that:
a) enums can be converted with automatic type deduction
b) enums can be converted with explicit dtype
c) integers can be converted to enums with explicit dtype
Checklist
Documentation
DALI team only
Requirements
REQ IDs: N/A
JIRA TASK: N/A