-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
logical_xor #22994
logical_xor #22994
Conversation
Thanks for contributing to Ivy! 😊👏 |
Hey @bipinKrishnan! Are there any issues with this pull request? |
Hey @roudik , thanks for the contribution! |
ivy-gardener |
Dear @bipinKrishnan, confused about whether there is still anything to be fixed as the tests show as passed. We should be able to close this PR then? |
@@ -437,6 +437,10 @@ def logical_not_(self): | |||
def logical_or(self, other): | |||
return torch_frontend.logical_or(self, other) | |||
|
|||
@with_unsupported_dtypes({"2.0.1 and below": ("bfloat16",)}, "torch") |
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.
Seems like the tests are failing due to an unsupported dtype not listed here. You could check the tests and add the unsupported dtype here.
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.
Which tests are failing? To me it shows that only run_test (1) is failing which should be ignored as per the instructions above?
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.
Not sure what you mean by unsupported dtype not listed, do you mean to remove the line 440? where do you want me to add unsupported dtype?
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.
Hey, you could check the "run_tests (1)` section of the CI and open up "Determine and Run tests" section to see the failing tests.
@bipinKrishnan Doesn't it say above that we can feel free to ignore run_tests? The display_test_results check shows as passed. |
Hey @roudik, its not like that actually. If you read carefully, you will see that "display_test_results" checks for newly introduced failures, so if you open it and check "New failures introduced" you can see that the tests you added are failing. If you need to look into the details of why its failing, you can check "run_tests (1)" which contains the detailed output/traceback of your test. Hope that makes sense. |
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.
PR Compliance Checks
Thank you for your Pull Request! We have run several checks on this pull request in order to make sure it's suitable for merging into this project. The results are listed in the following section.
Protected Branch
In order to be considered for merging, the pull request changes must not be implemented on the "main" branch. This is described in our Contributing Guide. We are closing this pull request and we would suggest that you implement your changes as described in our Contributing Guide and open a new pull request.
Conventional Commit PR Title
In order to be considered for merging, the pull request title must match the specification in conventional commits. You can edit the title in order for this check to pass.
Most often, our PR titles are something like one of these:
- docs: correct typo in README
- feat: implement dark mode"
- fix: correct remove button behavior
Linting Errors
- Found type "null", must be one of "feat","fix","docs","style","refactor","perf","test","build","ci","chore","revert"
- No subject found
This reverts commit 960fab3.
Dear @bipinKrishnan, now that makes sense, but I am not quite sure about what to add to the line with unsupported_dtypes. The working functions logical_or and logical_and both just use bfloat16, and logical_not doesn't even include the line about unsupported dtypes. I have noticed about the failed test mentioning about uint16, but when I tried adding it to the unsupported_dtypes line [...("bfloat16", "uint16",)...] I got even more errors than before. |
PR Description
Related Issue
Checklist
Socials: