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 dropout3d function and test for PaddlePaddle frontend #18924

Merged
merged 3 commits into from
Aug 14, 2023

Conversation

sachelsout
Copy link
Contributor

Close #18182

@sachelsout
Copy link
Contributor Author

All the test cases passing locally for dropout3d function !! 😄

image

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

Hi @danielmunioz , could you please review my PR ? 😃

@danielmunioz
Copy link
Contributor

danielmunioz commented Jul 13, 2023

Hey! @sachelsout! Thanks for contributing!!

The PR looks good but the tests seems to be failing on numpy, torch and paddle backends, locally at least 🤔, could you please fix the merge conflicts and confirm if the tests are passing again?

Thanks!

@sachelsout
Copy link
Contributor Author

Hey! @sachelsout! Thanks for contributing!!

The PR looks good but the tests seems to be failing on numpy, torch and paddle backends, locally at least 🤔, could you please fix the merge conflicts and confirm if the tests are passing again?

Thanks!

Hi @danielmunioz , I fixed the merge conflicts but after merging the base branch into my branch, the tests are failing, as you have mentioned. But the tests were passing successfully before, as seen in the screenshot I've uploaded.

I've followed the official PaddlePaddle docs for implementing dropout3d function and relevant test function. The tests are failing due to AssertionError, but I've written correct code for dropout3d.

Could you please guide me where am I going wrong. My guess is, there might have been some changes in the backend of these frameworks, which is leading to this error.

@danielmunioz
Copy link
Contributor

danielmunioz commented Jul 16, 2023

Hey @sachelsout, Yeah, it seems that there are still some merge errors popping up, and yeah the paddle test seems to be failing due to an assertion error, the results from the paddle library do not match with the results of the frontend implementation, the other but the other ones seem to be an hypothesis error, I'll take a look at those, and I'll get back to you, could you check the assertion one (find what's different) and fix the merge errors please?

@ivy-seed ivy-seed added the Stale label Aug 1, 2023
@ivy-seed
Copy link

ivy-seed commented Aug 1, 2023

This PR has been labelled as stale because it has been inactive for more than 7 days. If you would like to continue working on this PR, then please add another comment or this PR will be closed in 7 days.

@sachelsout
Copy link
Contributor Author

Hi @danielmunioz , apologies for the delay ! I resolved merge conflicts, updated the paddle to 2.5.1 in function definition for dropout3d, made relevant changes in the test function as well. All the test cases are passing locally.

image

@danielmunioz
Copy link
Contributor

Hey @sachelsout! Thanks for fixing the issues!

Yeah, all test seem to be passing locally
LGTM, Merging now!

Thanks for contributing!

@danielmunioz danielmunioz merged commit 93237f1 into ivy-llc:main Aug 14, 2023
132 of 133 checks passed
@sachelsout
Copy link
Contributor Author

Hi @danielmunioz , thank you for your guidance and review throughout the issue !! ❤️

@sachelsout sachelsout deleted the paddle_dropout3d branch August 14, 2023 12:29
This pull request was closed.
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 Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dropout3d
4 participants