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 checkfail with -ve perm values in transpose_op.cc #63064

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

SuryanarayanaY
Copy link
Collaborator

Passing negative values to argument perm in tf.raw_ops.Transpose will cause checkfail and coredump error. Hence validation proposed to check and raise exception.

Ref issue #62995

Passing negative values to argument perm in tf.raw_ops.Transpose will cause checkfail and coredump error.
Hence validation proposed to check and raise exception.

Ref issue #62995
@google-ml-butler google-ml-butler bot added the size:S CL Change Size: Small label Feb 27, 2024
@gbaned gbaned added prtype:bugfix PR to fix a bug comp:core issues related to core part of tensorflow labels Feb 28, 2024
@gbaned gbaned added this to Assigned Reviewer in PR Queue via automation Feb 28, 2024
@gbaned gbaned requested a review from sagunb February 28, 2024 04:32
@google-ml-butler google-ml-butler bot added the awaiting review Pull request awaiting review label Feb 28, 2024
Copy link
Member

@sagunb sagunb left a comment

Choose a reason for hiding this comment

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

Thank you for the changes! Just some minor comments to adhere to the c++ style guide.

tensorflow/core/kernels/transpose_op.cc Outdated Show resolved Hide resolved
tensorflow/core/kernels/transpose_op.cc Outdated Show resolved Hide resolved
PR Queue automation moved this from Assigned Reviewer to Reviewer Requested Changes Feb 28, 2024
Updated as suggested to adhere c++ standard
@gbaned
Copy link
Contributor

gbaned commented Mar 6, 2024

Hi @SuryanarayanaY Can you please check @mihaimaruseac's comments ? Thank you!

@gbaned gbaned added stat:awaiting response Status - Awaiting response from author and removed awaiting review Pull request awaiting review labels Mar 6, 2024
Local variable created outside loop.
@SuryanarayanaY
Copy link
Collaborator Author

Done the changes proposed.

@google-ml-butler google-ml-butler bot removed the stat:awaiting response Status - Awaiting response from author label Mar 6, 2024
Copy link
Collaborator

@mihaimaruseac mihaimaruseac left a comment

Choose a reason for hiding this comment

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

Thank you

@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Mar 6, 2024
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Mar 6, 2024
Updated TF error wrapper with absl
@google-ml-butler google-ml-butler bot removed the ready to pull PR ready for merge process label Mar 8, 2024
@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Mar 8, 2024
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Mar 8, 2024
@gbaned gbaned added ready to pull PR ready for merge process and removed ready to pull PR ready for merge process labels Mar 18, 2024
Replaced if check with OP_REQUIRES as the function is void type.
@google-ml-butler google-ml-butler bot removed the ready to pull PR ready for merge process label Mar 19, 2024
@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Mar 19, 2024
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Mar 19, 2024
@gbaned
Copy link
Contributor

gbaned commented Apr 16, 2024

Hi @SuryanarayanaY Can you please check @mihaimaruseac's comments ? Thank you!

@gbaned gbaned added stat:awaiting response Status - Awaiting response from author and removed ready to pull PR ready for merge process labels Apr 16, 2024
@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Apr 18, 2024
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Apr 18, 2024
Copy link

github-actions bot commented May 3, 2024

This PR is stale because it has been open for 14 days with no activity. It will be closed if no further activity occurs. Thank you.

@github-actions github-actions bot added the stale This label marks the issue/pr stale - to be closed automatically if no activity label May 3, 2024
@SuryanarayanaY
Copy link
Collaborator Author

Not stale. The PR has been amended.

@google-ml-butler google-ml-butler bot removed stale This label marks the issue/pr stale - to be closed automatically if no activity stat:awaiting response Status - Awaiting response from author labels May 3, 2024
@mihaimaruseac
Copy link
Collaborator

Not changed since last approval. It still fails internally

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:core issues related to core part of tensorflow prtype:bugfix PR to fix a bug ready to pull PR ready for merge process size:S CL Change Size: Small
Projects
PR Queue
  
Reviewer Requested Changes
Development

Successfully merging this pull request may close these issues.

None yet

5 participants