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

The if statement refers to bias.shape.length, but the error message refers to kernel.shape.length #8247

Closed
set5u opened this issue Apr 16, 2024 · 6 comments

Comments

@set5u
Copy link

set5u commented Apr 16, 2024

`${kernel.shape.length} instead`);

Shouldn't bias.shape.length be output as an error message here?

@gaikwadrahul8
Copy link
Contributor

Hi, @set5u

Thank you for bringing this issue to our attention and as per my current understanding, The if statement checks if bias is not null and its shape.length (number of dimensions) is not equal to 1. This is the correct check to ensure a valid bias tensor for a 1D convolution.However, the error message incorrectly references kernel.shape.length when it should be referencing bias.shape.length.

The following error message would be more accurate and clearly indicates that the issue lies with the number of dimensions in the bias tensor, not the kernel.

if (bias != null && bias.shape.length !== 1) {
      throw new ValueError(
          `The bias for a conv1dWithBias operation should be 1, but is ` +
          `${bias.shape.length} instead`);
    }

Would you like to submit a PR to take care of this issue from your end now so our developer team will review your PR and will take appropriate action from their end ? if not I'll go ahead and submit a PR from my end to take care of this error message issue.

Thank you for your cooperation and patience.

@set5u
Copy link
Author

set5u commented Apr 16, 2024

Could you please submit a PR from your end?

@gaikwadrahul8
Copy link
Contributor

Alright, I'll go ahead submit PR to take care of this issue. Thank you.

@gaikwadrahul8
Copy link
Contributor

Hi, @set5u

I have submitted a PR #8248 and this issue will be taken care once that PR got merged. Thank you.

@gaikwadrahul8
Copy link
Contributor

Hi, @set5u

I see that Pull Request #8248 has now been merged, which addresses the issue reported here. Therefore, I'm closing this ticket. Should you encounter any further problems or require additional assistance, please don't hesitate to create a new issue. We're always happy to help!

Thank you for your cooperation and patience.

Copy link

Are you satisfied with the resolution of your issue?
Yes
No

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants