ivy
ivy copied to clipboard
Transpile-AI
Add batch_norm function to paddle frontend
Hey @melch-inno!, Thanks for contributing!
It looks like you have some misnamed and some missing arguments in your function, could you please take a look at those?, a frontend implementation should resemble its original counterpart as closely as possible
Also it looks like you added a new file named run_tests_CLI/test_paddle_frontend.sh could you please remove that as well, the PR should ideally only contain modifications relevant to the issue at hand :)
Thanks!!
Hi @danielmunioz , the Ivy paddle backend already implements some of the features from the original Paddle batch_norm. Could we extend any other function statements at the backend if needed?
ivy paddle backend batch_norm function
Thanks again for the feedback 👍 :)
Hey @melch-inno, so it seems that you are using eps instead of epsilon in the frontend testing helper and so the function cannot properly recognize the argument name, argument names in the helper should match the ones in the frontend function so that it can work properly (checkout the frontend tests guide for more info).
Also the out argument of ivy.batch_norm is just an optional for output arrays, for writing the result to, paddle batch_norm doesn't seem to utilize it so there is no need to pass it into the ivy function.
Same goes for name and use_global_stats, these two are not directly being used in the function, while we need to keep them as arguments so as to not break frontend compatibility we can remove them from the function itself, check the unused arguments section of the deep dive, as well as the frontend guide itself which contains useful information about frontend functions.
If you have any questions please feel free to reach out! Thanks
Hi @danielmunioz , updates pushed. Pls, check when you are available. thanks~
Hi @theRealBird , Pls kindly check this PR when you are available. Thanks ~
Hey @melch-inno! Could you please update the supported version to 2.5.0? and update the dtypes accordingly if they need to? After that could you please run the tests and make sure they're passing?
Thanks!
Hey! @melch-inno, Sorry but your recent changes seem to be altering the backends, the backend implementations don't follow the structure of the original frameworks, they're more focused on functionality and unifying the behavior of the functions across all backends, so updating them to look like their counterparts is not necessary
Could you please remove the changes made in the backends? (Also, as per ivy rules, a PR needs to only contain modifications related to the issue at hand, so for example, in this case anything other than changes related to paddle_front batch_norm should go into another PR)
Please feel free to ask away! Thanks! :)
Hey! @melch-inno, Sorry but your recent changes seem to be altering the backends, the backend implementations don't follow the structure of the original frameworks, they're more focused on functionality and unifying the behavior of the functions across all backends, so updating them to look like their counterparts is not necessary
Could you please remove the changes made in the backends? (Also, as per ivy rules, a PR needs to only contain modifications related to the issue at hand, so for example, in this case, anything other than changes related to paddle_front batch_norm should go into another PR)
Please feel free to ask away! Thanks! :)
Hi @danielmunioz, I understand, but we might have to create a new ticket/task to make those changes at the backend because the paddle backend does not allow positional-only arguments passed as keyword arguments. This PR can be put on hold for those changes to be done before I can continue. see the exception below for reference; thanks ~
I'll take a look at those backend errors, but right now there seems to still be a merge error as well as an unexpected argument in the tests, seems that the y arguments is unused there, could you please fix those while I check the backend?
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.
Hey @melch-inno! Could you please remove the usage of the paddle package? the frontend is meant to be used using frontend arrays, so casting to paddle arrays is not necessary!
Hey @BilkisAhmed! This task is currently a work in progress, but if you'd like to contribute to ivy please check out the Open Tasks that we have available!