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

Implemented the following Jax Frontend functions: #5575

Closed
wants to merge 6 commits into from

Conversation

AlphaPro06
Copy link
Contributor

@AlphaPro06 AlphaPro06 commented Oct 11, 2022

Implemented jax numpy frontend functions

@ivy-leaves ivy-leaves added the JAX Frontend Developing the JAX Frontend, checklist triggered by commenting add_frontend_checklist label Oct 11, 2022
@AlphaPro06 AlphaPro06 changed the title Implemented the following Jax Frontend functions: #5493 Implemented the following Jax Frontend functions: Oct 11, 2022
@AlphaPro06 AlphaPro06 changed the title #5493 Implemented the following Jax Frontend functions: Issue Number: #5493 Implemented the following Jax Frontend functions: Oct 11, 2022
.idea/ivy.iml Outdated Show resolved Hide resolved
.idea/misc.xml Outdated Show resolved Hide resolved
Copy link
Contributor

@YushaArif99 YushaArif99 left a comment

Choose a reason for hiding this comment

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

Hi @AlphaPro06

The PR looks great overall. Could you kindly make the requested changes? Also, the lint tests seem to be failing. Could you kindly fix that as well 😀

Copy link
Contributor Author

@AlphaPro06 AlphaPro06 left a comment

Choose a reason for hiding this comment

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

Made the requested changes, I don't understand the file in .idea/runConfiguration/_template__of_py_test.xml have the exact same file contente

Copy link
Contributor Author

@AlphaPro06 AlphaPro06 left a comment

Choose a reason for hiding this comment

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

I've made the requested changes, and it passed the lint test as well. Can you please approve the changes? 😃

@AlphaPro06 AlphaPro06 mentioned this pull request Oct 13, 2022
@AlphaPro06 AlphaPro06 changed the title Issue Number: #5493 Implemented the following Jax Frontend functions: Implemented the following Jax Frontend functions: Oct 13, 2022
@AlphaPro06 AlphaPro06 closed this Oct 13, 2022
@AlphaPro06 AlphaPro06 reopened this Oct 13, 2022
Copy link
Contributor Author

@AlphaPro06 AlphaPro06 left a comment

Choose a reason for hiding this comment

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

Solved merge conflicts, hopefully now the code can merge. @YushaArif99 Please review and merge if my contribution looks good to you!

Copy link
Contributor Author

@AlphaPro06 AlphaPro06 left a comment

Choose a reason for hiding this comment

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

fixed the issues described

@AlphaPro06 AlphaPro06 closed this Oct 14, 2022
@AlphaPro06 AlphaPro06 reopened this Oct 14, 2022
@ivy-leaves ivy-leaves added the TensorFlow Frontend Developing the TensorFlow Frontend, checklist triggered by commenting add_frontend_checklist label Oct 14, 2022
@AlphaPro06
Copy link
Contributor Author

@YushaArif99 hi I made the changes you've requested, please let me know if there is anything else I would need to change. If there is nothing else for me to change, please approve and merge my pull request as soon as possible!

@handle_cmd_line_args
@given(
dtype_and_x=helpers.dtype_and_values(
available_dtypes=helpers.get_dtypes("integer"), num_arrays=1
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you kindly explain as to why you're using "integers" and "floats" separately for cumsum and cumprod but using "valid" for asarray. Is valid not working with the other 2?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah "valid" wasn't really working on cumsum and cumprod, it's also why I tried using "integers" and "floats" differently but now I think "valid" is working now for cumsum and prod but I am getting the MultipleFailures exception where Hypothesis found 2 distinct failures I've mentioned in my new comment.

Copy link
Contributor

@YushaArif99 YushaArif99 left a comment

Choose a reason for hiding this comment

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

Hi, @AlphaPro06. Could you kindly review the changes I have requested? Also, kindly resolve the merge conflicts so that the CI tests can run. I have tried running the tests on codespaces and they are failing for all 3 functions. Kindly look into it as well. Thanks!😊

@AlphaPro06 AlphaPro06 reopened this Oct 17, 2022
@AlphaPro06
Copy link
Contributor Author

AlphaPro06 commented Oct 17, 2022

@YushaArif99 Hi, I hope all is well. I tried to really debug the issues I have facing. So I tried using valid dtypes for both cumsum and cumprod. I really don't understand how else can I fix the Hypothesis errors I am recieving, I apologize but I really feel I don't know what else can I do to fix these errors. Do you have any suggestions that could help? Thank you!

Edit: Just to let you know there are 2 distinct failures now as opposed to 3

Screen Shot 2022-10-16 at 11 59 52 PM
Screen Shot 2022-10-17 at 12 01 27 AM

@AlphaPro06 AlphaPro06 closed this Oct 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
JAX Frontend Developing the JAX Frontend, checklist triggered by commenting add_frontend_checklist TensorFlow Frontend Developing the TensorFlow Frontend, checklist triggered by commenting add_frontend_checklist
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants