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 scalar handling in GPU cast #3924

Merged
merged 1 commit into from
May 23, 2022
Merged

Conversation

mzient
Copy link
Contributor

@mzient mzient commented May 21, 2022

Signed-off-by: Michał Zientkiewicz mzient@gmail.com

Category:

Bug fix (non-breaking change which fixes an issue)

Description:

Scalars were not properly handled in operator cast - there was an attempt to collapse the shape, which failed catastrophically (assertion in Debug build and access violation in Release) if the input shape is 0D, in which case it needs to be artificially expanded.
This PR adds special handling for that case.

Additional information:

Affected modules and functionalities:

Operator cast.

Key points relevant for the review:

N/A

TODO (separate PR): Add proper tests for operator cast.

Checklist

Tests

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

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

Signed-off-by: Michał Zientkiewicz <mzient@gmail.com>
@mzient mzient added the important-fix Fixes an important issue in the software or development environment. label May 21, 2022
@mzient
Copy link
Contributor Author

mzient commented May 21, 2022

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [4899734]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [4899734]: BUILD PASSED

@stiepan
Copy link
Member

stiepan commented May 23, 2022

I get that adding thorough testing may be out of scope, but how about including here a single test case that would explode without the fix and passes otherwise?

@stiepan stiepan self-assigned this May 23, 2022
@mzient
Copy link
Contributor Author

mzient commented May 23, 2022

I get that adding thorough testing may be out of scope, but how about including here a single test case that would explode without the fix and passes otherwise?

I don't think it's worth wasting a perfectly good CI run ;)

@jantonguirao jantonguirao self-assigned this May 23, 2022
@jantonguirao
Copy link
Contributor

I get that adding thorough testing may be out of scope, but how about including here a single test case that would explode without the fix and passes otherwise?

I agree that such test is a good idea.

@mzient mzient merged commit aa0196f into NVIDIA:main May 23, 2022
stiepan pushed a commit that referenced this pull request May 24, 2022
Don't try to collapse 0D shape - reinterpret the data as 1D 1-element instead.

Signed-off-by: Michał Zientkiewicz <mzient@gmail.com>
cyyever pushed a commit to cyyever/DALI that referenced this pull request Jun 7, 2022
Don't try to collapse 0D shape - reinterpret the data as 1D 1-element instead.

Signed-off-by: Michał Zientkiewicz <mzient@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
important-fix Fixes an important issue in the software or development environment.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants