Skip to content

Critical typo: squeeze the y_pred tensor even when it’s the same rank and shape as the y_pred tensor should be == instead of != #719

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

Closed
wants to merge 1 commit into from

Conversation

rodriguesk
Copy link

Current behavior will squeeze the y_pred tensor even when it’s the same rank and shape as the y_pred tensor.

The issue is that the code is written wrong. Read the code comments and what it’s supposed to do (only squeeze when the ranks differ by exactly 1 and not squeeze for situations of equal rank). Yet the code’s if statements will squeeze whenever the ranks are equal, which is wrong from what I can tell.

the line: if (y_pred_rank - y_true_rank != 1) or y_pred_shape[-1] == 1: should be:
if (y_pred_rank - y_true_rank == 1) or y_pred_shape[-1] == 1:

… and shape as the y_pred tensor should be == instead of !=

Current behavior will squeeze the y_pred tensor even when it’s the same rank and shape as the y_pred tensor.

The issue is that the code is written wrong. Read the code comments and what it’s supposed to do (only squeeze when the ranks differ by exactly 1 and not squeeze for situations of equal rank). Yet the code’s if statements will squeeze whenever the ranks are equal, which is wrong from what I can tell.

the line: if (y_pred_rank - y_true_rank != 1) or y_pred_shape[-1] == 1: 
should be:
if (y_pred_rank - y_true_rank == 1) or y_pred_shape[-1] == 1:
Copy link

google-cla bot commented Jan 15, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@tilakrayal
Copy link
Collaborator

@rodriguesk,
This PR is a duplicate of the other PR #715 which is under process. Could you please confirm and feel free to close the PR. Thank you for the contribution!

@rodriguesk
Copy link
Author

rodriguesk commented Jan 22, 2024 via email

@tilakrayal
Copy link
Collaborator

@rodriguesk,
Closing this PR as the duplicate. Thank you!

@tilakrayal tilakrayal closed this Jan 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants