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

num_ivy_arrays_on_dev #1451

Merged
merged 18 commits into from
Jun 19, 2022
Merged

num_ivy_arrays_on_dev #1451

merged 18 commits into from
Jun 19, 2022

Conversation

davidlee1102
Copy link
Contributor

No description provided.

@simonetgordon
Copy link
Contributor

Hi David,

Thanks for making a new PR!

test-numpy-style-docstrings should be passing, so checkout the 'details' for this.

@davidlee1102
Copy link
Contributor Author

Hi Simone,
I have fixed this error, please take a look on that 🙇
Thank you!
David Lee

@davidlee1102
Copy link
Contributor Author

Hi Simone,
I did see that all the unsuccessful did not come from my code, could you recheck, I did see that was happened on others' file, not my code
Thank you!
David Lee

@simonetgordon
Copy link
Contributor

Hi David,

Could you pull from master? Some fixes have been introduced for some of the tests, thanks!

@davidlee1102
Copy link
Contributor Author

Hi Simone,
Please check my code again
Thank you!
David

@simonetgordon
Copy link
Contributor

Hi David,

So currently your examples are failing the docstring tests. Have you tried running this function using the master version of ivy? When I do, I need to write the examples in a different manner to the way you have written them and obtain different results ( the # numbers is what was printed out)

import ivy #ivy master

ivy.set_backend('numpy')

x = ivy.array([-1,0,5.2])
y = ivy.num_ivy_arrays_on_dev(ivy.default_device())
print(y) # 1

x = ivy.array([-1,0,5.2])
y = ivy.array([-1,0,5.2,6])
z = ivy.num_ivy_arrays_on_dev(ivy.default_device())
print(z) # 2

x = ivy.Container(a=ivy.array([0., 1., 2.]), b=ivy.array([3., 4., 5.]))
z = ivy.num_ivy_arrays_on_dev(ivy.default_device())
print(z) # 3

Make sure to follow the Contributer Guide closely, particularly for containers.

@davidlee1102
Copy link
Contributor Author

davidlee1102 commented Jun 16, 2022

Hi Simone,
I am confused about the result came from the code running
Here is the example I found and understood on ivy/master

   Examples
    --------
    >>> x = ivy.array([-1,0,5.2])
    >>> y = ivy.dev(x)
    >>> z = ivy.num_ivy_arrays_on_dev(y)
    >>> print(z)
    2

And here below was your code

import ivy #ivy master

ivy.set_backend('numpy')

x = ivy.array([-1,0,5.2])
y = ivy.num_ivy_arrays_on_dev(ivy.default_device())
print(y) # 1

x = ivy.array([-1,0,5.2])
y = ivy.array([-1,0,5.2,6])
z = ivy.num_ivy_arrays_on_dev(ivy.default_device())
print(z) # 2

x = ivy.Container(a=ivy.array([0., 1., 2.]), b=ivy.array([3., 4., 5.]))
z = ivy.num_ivy_arrays_on_dev(ivy.default_device())
print(z) # 3


About the result, I think somewhere was wrong, could you recheck on that? 🙇
`
>>> x = ivy.array([-1,0,5.2])
>>> z = ivy.num_ivy_arrays_on_dev(ivy.dev(x))
>>> print(z)
2

`
The original told that it was 2, not 1, could you please check the ivy/master branch to make sure again, I am sorry for making a lot of comments, because of confusing 🙇
Thank you,
David Lee

@simonetgordon
Copy link
Contributor

Hi David,

The original example was failing the tests too. My code was run on a python script importing the master branch of ivy so it's not wrong. Most likely, there have been changes to the function since the original example was written.

You can also run my code by making a new file in your local clone of ivy and running it on the command line/terminal.

@davidlee1102
Copy link
Contributor Author

Hi Simone,
I have fixed base on your suggestion, I dont understand too much about this so I was following on your idea
Hope that it can work well
Thank you,
David

@simonetgordon
Copy link
Contributor

Hi David,

It would be best if you could run the code yourself and use this to write the docstring examples. Are you using PyCharm or VSCode to view and edit the ivy source code?

@davidlee1102
Copy link
Contributor Author

HI Simone,
I will edit it follow on my code and example
About the coding ide, I am using Pycharm for coding Ivy
I will notify you after finishing the next commit
Thank you!
David

@davidlee1102
Copy link
Contributor Author

Hi Simone,
I have tried and the code was running successful, however, I am confused about:

  1. The name of issue was: num_arrays_on_dev | However the def name on device.py was num_ivy_arrays_on_dev, could you please check again
  2. The read docs seem to be old, I have put ivy.set_framework('numpy') instead of ivy.set_backend('numpy'), It may give little bit confused
  3. The result I have tried was the same with the examples, it was different with your result
    I hope that you can check it and let me know about that
    Thank you,
    David

@simonetgordon
Copy link
Contributor

Hi David,

  1. the correct function is now num_ivy_arrays_on_dev. The version without 'ivy' was probably an old iteration of the function.
  2. ivy.set_backend('numpy') works in scripts which have imported ivy cloned recently from the master branch. If ivy.set_backend('numpy') does not work for you, you are not importing the master branch ivy, but rather the stable ivy.

As I mentioned before, you can create a new .py script under the /ivy directory in PyCharm and import ivy in this script. As long as your local fork is up-to-date with the ivy master branch, you should be able to run ivy.set_backend('numpy') and the code I have written above successfully.

Kind Regards,

Simone

@davidlee1102
Copy link
Contributor Author

davidlee1102 commented Jun 17, 2022

Hi Simone,
I have fixed and It is currently running well, please check on my code 🙇
About the result, I have found that your result was wrong in one function I have attached below

`
x = ivy.Container(a=ivy.array([0., 1., 2.]), b=ivy.array([3., 4., 5.]))
z = ivy.num_ivy_arrays_on_dev(ivy.default_device())
print(z) # 3
`

The result should be 2, not 3
I have also update for container and for both native_array and array too, hope that it can work well
Thank you for helping me to contribute to the ivy
Best regards,
David

@simonetgordon
Copy link
Contributor

Hi David,

Glad you got it working! And indeed it should have been #2 instead of #3, I mistyped but did get 2 as the answer, my mistake.

Small thing to fix with the examples: the set_backend line is not needed, as it should be tested for all backends.

@davidlee1102
Copy link
Contributor Author

Hi Simone,
I have fixed, so hope that it's finally for this function 😸
Thank you for supporting me,
David

@simonetgordon
Copy link
Contributor

Hi David,

There is the warning:
UserWarning: Output is unequal to the docstrings output: num_ivy_arrays_on_dev
appearing in the 'details' of the 'Run Docstring Tests'.

Please double check all the outputs to the functions is correct. Also, the format is important. The { } brackets in the last few examples may be causing this warning.

Kind Regards,

Simone

@davidlee1102
Copy link
Contributor Author

Hi Simeone,
I have checked all the function, all the examples were working well and the result of this was correct;
Could you recheck for me? I dont know where caused this warning
Thank you,
David

@davidlee1102
Copy link
Contributor Author

Hello @simonetgordon ,
I have check with the other guys, and I did see that he/she also had the same error on this PR
UserWarning: Output is unequal to the docstrings output:
So could you please recheck my code again
Thank you,
David

@simonetgordon
Copy link
Contributor

Hi David,

Sorry, I don't follow what you mean. The user warning shouldn't be there. If there is, there is an issue with the docstring example not matching the output of the function when it's run. Could you please try removing the { } brackets in the code and see if that gets the test passing without a warning?

@davidlee1102
Copy link
Contributor Author

Hi Simone,
I have fixed, It was warning with {} as the same as you told me, thank you 😺
Could you check that for me again
Thank you!
David

@simonetgordon
Copy link
Contributor

Hi David,

That's great! Ready to merge 😊 Thanks again for the contribution.

Simone

@simonetgordon simonetgordon merged commit 8e45295 into ivy-llc:master Jun 19, 2022
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.

4 participants