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

Added bilinear function in ivy.frontend.paddle.nn.functional.common.py #21371

Closed
wants to merge 14 commits into from

Conversation

MuhammadTalha-10
Copy link

@MuhammadTalha-10 MuhammadTalha-10 commented Aug 6, 2023

close #21366

@github-actions
Copy link
Contributor

github-actions bot commented Aug 6, 2023

Thanks for contributing to Ivy! 😊👏
Here are some of the important points from our Contributing Guidelines 📝:
1. Feel free to ignore the run_tests (1), run_tests (2), … jobs, and only look at the display_test_results job. 👀 It contains the following two sections:
- Combined Test Results: This shows the results of all the ivy tests that ran on the PR. ✔️
- New Failures Introduced: This lists the tests that are passing on master, but fail on the PR Fork. Please try to make sure that there are no such tests. 💪
2. The lint / Check formatting / check-formatting tests check for the formatting of your code. 📜 If it fails, please check the exact error message in the logs and fix the same. ⚠️🔧
3. Finally, the test-docstrings / run-docstring-tests check for the changes made in docstrings of the functions. This may be skipped, as well. 📚
Happy coding! 🎉👨‍💻

@ivy-leaves ivy-leaves added the Paddle Frontend Developing the Paddle Frontend, checklist triggered by commenting add_frontend_checklist label Aug 6, 2023
@ivy-leaves
Copy link

If you are working on an open task, please edit the PR description to link to the issue you've created.

For more information, please check ToDo List Issues Guide.

Thank you 🤗

@MuhammadTalha-10 MuhammadTalha-10 changed the title Added bilinear function in ivy.frontend.paddle.nn.functional.common.py https://github.com/unifyai/ivy/issues/21366 Aug 6, 2023
Copy link
Contributor

@AryanSharma21 AryanSharma21 left a comment

Choose a reason for hiding this comment

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

Hello, @MuhammadTalha-10 ,Kindly add the close #issue_number in the PR description, so that the pull request gets linked to the issue that you have just created. Doing this, will automatically close the issue as completed when the pull request gets merged.

Also, while you have added the implementation for the function, you should also add the test function in the corresponding test file.
Thanks

@MuhammadTalha-10 MuhammadTalha-10 changed the title https://github.com/unifyai/ivy/issues/21366 https://github.com/unifyai/ivy/issues/21366. close #21366 Aug 6, 2023
@MuhammadTalha-10 MuhammadTalha-10 changed the title https://github.com/unifyai/ivy/issues/21366. close #21366 Added bilinear function in ivy.frontend.paddle.nn.functional.common.py Aug 6, 2023
@MuhammadTalha-10
Copy link
Author

MuhammadTalha-10 commented Aug 6, 2023

Also, while you have added the implementation for the function, you should also add the test function in the corresponding test file. Thanks

In ivy_test directory?

@AryanSharma21
Copy link
Contributor

Yes the description is correct as the issue is now linked , and you should implement test in the ivy test directory. please find the correct frontend test file.

@MuhammadTalha-10
Copy link
Author

Yes the description is correct as the issue is now linked , and you should implement test in the ivy test directory. please find the correct frontend test file.

I will do it as soon as possible thank you for guidance.

@MuhammadTalha-10
Copy link
Author

Yes the description is correct as the issue is now linked , and you should implement test in the ivy test directory. please find the correct frontend test file.

I have committed the final changes

@AryanSharma21
Copy link
Contributor

@MuhammadTalha-10 , looked at the test logs, It seems that there is some issue with the test configuration.

E fixture 'dtype_x_weight_bias' not found

please recheck it once and do try to run the test locally as well.
Besides , we can wait for review from maintainer @aparajith21 , to understand better

@MuhammadTalha-10
Copy link
Author

@MuhammadTalha-10 , looked at the test logs, It seems that there is some issue with the test configuration.

E fixture 'dtype_x_weight_bias' not found

please recheck it once and do try to run the test locally as well.
Besides , we can wait for review from maintainer @aparajith21 , to understand better

Sure, I will try to fix this issue

@MuhammadTalha-10
Copy link
Author

@MuhammadTalha-10 , looked at the test logs, It seems that there is some issue with the test configuration.

E fixture 'dtype_x_weight_bias' not found

please recheck it once and do try to run the test locally as well. Besides , we can wait for review from maintainer @aparajith21 , to understand better

I tested it locally after making some changes and no error occurred there was just a warning "WARNING:root:This function includes the mixed partial function 'ivy.linear'. Please note that the returned data types may not be exhaustive. Please check the dtypes of ivy.linear for more details"

frontend,
test_flags,
):
dtype, x1, x2, weight, bias = dtype_x_weight_bias
Copy link
Contributor

@aparajith21 aparajith21 Aug 11, 2023

Choose a reason for hiding this comment

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

Please fetch the support dtypes for this test instead of reusing the earlier one for linear using the handle_frontend_test decorator similar to other tests to configure the test correctly!

@aparajith21
Copy link
Contributor

Thanks for contributing, sorry just checking! Could you comment add_frontend_checklist and tick off the items there please before I review!

@MuhammadTalha-10
Copy link
Author

Can you help me with these conflicts? I am committing changes in my own branch but there are conflicting files and I am unable to figure this out why is this happening

@@ -113,5 +113,14 @@ def linear(x, weight, bias=None, name=None):

@to_ivy_arrays_and_back
@with_supported_dtypes({"2.5.1 and below": ("float32", "float64")}, "paddle")
def dropout3d(x, p=0.5, training=True, data_format="NCDHW", name=None):
return ivy.dropout3d(x, p, training=training, data_format=data_format)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please don't remove any existing code to add yours!

@handle_frontend_test(
fn_tree="paddle.nn.functional.common.dropout3d",
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, please don't remove code to add tests!

@aparajith21
Copy link
Contributor

aparajith21 commented Aug 16, 2023

Can you help me with these conflicts? I am committing changes in my own branch but there are conflicting files and I am unable to figure this out why is this happening

Try pulling a fresh fork of ivy and then adding your changes, currently you seem to have removed a few lines of code that were present, please revert these, and can then review once you request. Thanks for contributing

@MuhammadTalha-10
Copy link
Author

I will try that, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Paddle Frontend Developing the Paddle Frontend, checklist triggered by commenting add_frontend_checklist
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bilinear
6 participants