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

Close #21810 #22256

Closed
wants to merge 5 commits into from
Closed

Close #21810 #22256

wants to merge 5 commits into from

Conversation

DevBhuyan
Copy link

@DevBhuyan DevBhuyan commented Aug 20, 2023

Closes #21810
#21810

From ToDo list: #14945
#14945

Update:

  1. Added transformer layer in ~/ivy/ivy/stateful/layers.py
  2. Added test test_transformer_layer in ~/ivy/ivy_tests/test_ivy/test_stateful/test_layers.py
  3. Added custom composite strategy to generate data (ref. transformer_data())

This is my first PR. I tried to make it as appropriate as possible. Please let me know if there are any modifications that you'd suggest. Thank you!😊

@github-actions
Copy link
Contributor

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 main, 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! 🎉👨‍💻

@yopknopixx
Copy link
Contributor

Hey, @DevBhuyan could you please link the open issue in the PR description?

@DevBhuyan
Copy link
Author

Sure @yopknopix. I have updated the initial comment accordingly. I've included the open issue #21810 as well as the corresponding ToDo list. Thank you!

@yopknopixx
Copy link
Contributor

Can you pull the latest changes from master? There seems to be some issue with the testing pipeline, no tests are being run. Thanks!

@DevBhuyan
Copy link
Author

Sure, I've synced my version of the fork again. I can see that one of the checks run_tests (1) (pull_request) has failed. This has been constantly failing for all of the previous commits. I guess it's safe to ignore that one, as mentioned here:

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. 👀........

I hope it helped. Please let me know if there is still an issue. Thank you 😇

@@ -5,6 +5,8 @@
from ivy.func_wrapper import handle_nestable
from ivy.stateful.initializers import GlorotUniform, Zeros
from ivy.stateful.module import Module
from ivy.stateful.norms import LayerNorm
import numpy as np
Copy link
Contributor

Choose a reason for hiding this comment

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

@DevBhuyan You should not use NumPy for this. Please use ivy functions.

Copy link
Author

Choose a reason for hiding this comment

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

Oops! My bad, I'll change it. I assumed the other way😅

@yopknopixx
Copy link
Contributor

@DevBhuyan it seems for some reason the tests are not running at all. As for run_tests (1) (pull_request) you're correct, but that's only when tests are running and failing.

@DevBhuyan
Copy link
Author

Oh I see, please let me know if there's something I need to do to help resolve the testing issue. Meanwhile I'll fix the code and commit again. Thank you 😇

@yopknopixx
Copy link
Contributor

Can you try creating a new branch on your fork from unifyai:main, implementing these changes, and creating a new PR?

@DevBhuyan
Copy link
Author

Sure! I'll do that then.

@DevBhuyan
Copy link
Author

Dear @yopknopixx, I have created the new branch DevBhuyan:secondary, committed the changes, and started a fresh pull request here. I have copy-pasted the description as it is:

Closes #21810 #21810

From ToDo list: #14945 #14945
....

Is there any action to be taken with this PR (current thread)?

@yopknopixx
Copy link
Contributor

Hey @DevBhuyan, we can close this PR. The tests seem to be working fine on the new one.

@yopknopixx yopknopixx closed this Aug 22, 2023
@DevBhuyan
Copy link
Author

Alright, thanks 😇. But I guess the new PR is being handled by someone else, is it?

@DevBhuyan DevBhuyan deleted the master branch August 25, 2023 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

transformer
2 participants