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 unravel_index #12876

Closed
wants to merge 5 commits into from
Closed

added unravel_index #12876

wants to merge 5 commits into from

Conversation

rajshukla1102
Copy link
Contributor

Close #10421

@rajshukla1102
Copy link
Contributor Author

@hirwa-nshuti hey I am trying to resolve unit test but for every backend test I am getting the error
AssertionError: The length of results from backend jax and ground truthframework numpy does not match can you help me in this?

@ivy-leaves ivy-leaves added the NumPy Frontend Developing the NumPy Frontend, checklist triggered by commenting add_frontend_checklist label Mar 21, 2023
@fnhirwa fnhirwa self-assigned this Mar 21, 2023
@fnhirwa
Copy link
Contributor

fnhirwa commented Mar 21, 2023

Hey @rajshukla1102

It is likely an issue with how we are handling 0-d arrays when testing I'll look into this tomorrow😊
Meanwhile, once other backend tests are passing I'll get the PR merged

Thanks

@rajshukla1102
Copy link
Contributor Author

@hirwa-nshuti Sure! Thankyou😊

Copy link
Contributor

@fnhirwa fnhirwa left a comment

Choose a reason for hiding this comment

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

Hey @rajshukla1102

The tests seems to be failing due to this tuple you introduced try to remove it and use ivy.unravel_index only

Thanks

Copy link
Contributor

@fnhirwa fnhirwa left a comment

Choose a reason for hiding this comment

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

Hey @rajshukla1102

The tests are failing due to dtype of returned indices which seems to be int32
You must change it to int64.

Thanks

@rajshukla1102
Copy link
Contributor Author

The tests are failing due to dtype of returned indices which seems to be int32 You must change it to int64.

you mean casting indices to int64?
actually I have tried to cast indices to int64 then passed indices to unravel_index but it didn't work although!

@fnhirwa
Copy link
Contributor

fnhirwa commented Mar 27, 2023

The tests are failing due to dtype of returned indices which seems to be int32 You must change it to int64.

you mean casting indices to int64? actually I have tried to cast indices to int64 then passed indices to unravel_index but it didn't work although!

I meant something like this:

ret = [x.astype("int64") for x in ivy.unravel_index(indices, shape)]
return tuple(ret)
   

@rajshukla1102
Copy link
Contributor Author

@hirwa-nshuti I have made changes!

Copy link
Contributor

@fnhirwa fnhirwa left a comment

Choose a reason for hiding this comment

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

Hey @rajshukla1102

All looks good to be merged, but you have changed .idea files 🤣
Would you remove those changes 🙂

Thanks

@rajshukla1102
Copy link
Contributor Author

@hirwa-nshuti I have deleted the file and also removed unused import! Please look at it once.
Thankyou🙂

@fnhirwa
Copy link
Contributor

fnhirwa commented Mar 29, 2023

Hey @rajshukla1102

if you check here the .idea changes are still there https://github.com/unifyai/ivy/pull/12876/files

@rajshukla1102
Copy link
Contributor Author

@hirwa-nshuti Due to some issue I have referenced a new PR to this. #13380

@rajshukla1102 rajshukla1102 deleted the unravelindexpr branch March 31, 2023 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NumPy Frontend Developing the NumPy Frontend, checklist triggered by commenting add_frontend_checklist
Projects
None yet
Development

Successfully merging this pull request may close these issues.

unravel_index
4 participants