Add reshape frontends.torch.indexing_slicing_joining_mutating_ops #2637

Creative-Ataraxia · 2022-08-04T14:12:44Z

link to issue: #2636

to frontends.torch.indexing_slicing_joining_mutating_ops

to test_frontends.test_indexing_slicing_joining_mutating_ops

juliagsy

Hey there! Thanks for contributing! As you chose reshape as your frontend function to work on, please only make changes on that function and its tests without anything else, thanks!

Creative-Ataraxia · 2022-08-06T02:01:29Z

Hey there! Thanks for contributing! As you chose reshape as your frontend function to work on, please only make changes on that function and its tests without anything else, thanks!
@juliagsy
Hi! thanks for the followup! The reason I had the others in there is because I've also claimed those tasks too. PRs: #2625, #2633; but I haven't got any feedbacks on those yet..

For PR formatting: Should I not include all the changes up to this point in each new branch?
Are what I had with reshape in this PR the correct way to do things? would love some feedbacks so I can improve future RPs

Creative-Ataraxia · 2022-08-06T02:16:49Z

@juliagsy I removed the other tasks currently in PR #2625, #2633 and left only reshape; please provide feedback on reshape, thanks :)

juliagsy · 2022-08-06T03:28:17Z

oh! I see, yeah, please do each function in different PRs! I'll take a look at reshape and provide a review soon! Thanks!

juliagsy

Hey there! Great work so far, there are a few changes required before it's good to go! Please request my review again when you're done! Thank you!

ivy/functional/frontends/torch/indexing_slicing_joining_mutating_ops.py

juliagsy · 2022-08-06T03:35:30Z

ivy_tests/test_ivy/test_frontends/test_torch/test_indexing_slicing_joining_mutating_ops.py

+
+# reshape
+@given(
+    xs_n_input_dtypes_n_unique_idx=_arrays_idx_n_dtypes(),


please use helpers.dtype_and_values as unique idx is not needed, and set the argument available_dtypes to:

available_dtypes=tuple( set(ivy_np.valid_float_dtypes).intersection( set(ivy_torch.valid_float_dtypes) ) )

juliagsy · 2022-08-06T03:37:53Z

ivy_tests/test_ivy/test_frontends/test_torch/test_indexing_slicing_joining_mutating_ops.py

+    with_out,
+    fw,
+):
+    xs, input_dtypes, unique_idx = xs_n_input_dtypes_n_unique_idx


once you've changed to use dtype_and_values, there will only be two variables - dtype, x = ... needed

juliagsy · 2022-08-06T03:39:04Z

ivy_tests/test_ivy/test_frontends/test_torch/test_indexing_slicing_joining_mutating_ops.py

+    fw,
+):
+    xs, input_dtypes, unique_idx = xs_n_input_dtypes_n_unique_idx
+    xs = [np.asarray(x, dtype=dt) for x, dt in zip(xs, input_dtypes)]


keep only np.asarray(x, dtype=...), extra square brackets for list casting is not required too

juliagsy · 2022-08-06T03:39:37Z

ivy_tests/test_ivy/test_frontends/test_torch/test_indexing_slicing_joining_mutating_ops.py

+        fw=fw,
+        frontend="torch",
+        fn_name="reshape",
+        tensors=xs,


please update parameters here to match parameters in torch.reshape, with variables mapped to their respective arguments

question:
the args of helpers.test_frontend_function() should be the expected args of the backend functions, in this case, since torch.reshape only asks for 2 args, input: Tensor, shape: Tuple(int), we would only include these two args for helpers.test_frontend_function(), correct?

use ivy.reshape which is the main alias

question: the args of helpers.test_frontend_function() should be the expected args of the backend functions, in this case, since torch.reshape only asks for 2 args, input: Tensor, shape: Tuple(int), we would only include these two args for helpers.test_frontend_function(), correct?

Creative-Ataraxia · 2022-08-06T07:22:00Z

@juliagsy I've updated per your instructions. I do have two questions if I may:

the args of helpers.test_frontend_function() should take the expected args of the backend functions, in this case, since torch.reshape only asks for 2 args, input: Tensor, shape: Tuple(int), we would only input these two args for helpers.test_frontend_function(), correct?
the returns of helpers.dtype_and_values() are dtype, values and shape; The values as returned is not yet a tensor correct? helpers.test_frontend_function() cast the values: List[int] to torch.tensors?

to match backends.torch.reshape's arguments

'ivy/functional/frontends/torch/indexing_slicing_joining_mutating_ops.py:8:1: E305 expected 2 blank lines after class or function definition, found 1'

replace all ' = ' with '='

1. undefined name 'ivy_np' 2. line too long (90 > 88 characters)

…ng_slicing_joining_mutating_ops

1. 31:1: E303 too many blank lines (3) 2. 37:1: E302 expected 2 blank lines, found 1 3. 42:1: W391 blank line at end of file

ivy_tests/test_ivy/test_frontends/test_tensorflow/test_tf_functions.py:196:9: E126 continuation line over-indented for hanging indent ivy_tests/test_ivy/test_frontends/test_tensorflow/test_tf_functions.py:197:9: E122 continuation line missing indentation or outdented ivy_tests/test_ivy/test_frontends/test_tensorflow/test_tf_functions.py:200:9: E122 continuation line missing indentation or outdented ivy_tests/test_ivy/test_frontends/test_tensorflow/test_tf_functions.py:201:9: E122 continuation line missing indentation or outdented ivy_tests/test_ivy/test_frontends/test_tensorflow/test_tf_functions.py:202:5: E122 continuation line missing indentation or outdented ivy_tests/test_ivy/test_frontends/test_tensorflow/test_tf_functions.py:203:5: E131 continuation line unaligned for hanging indent ivy_tests/test_ivy/test_frontends/test_tensorflow/test_tf_functions.py:204:5: E131 continuation line unaligned for hanging indent ivy_tests/test_ivy/test_frontends/test_tensorflow/test_tf_functions.py:206:5: E122 continuation line missing indentation or outdented ivy_tests/test_ivy/test_frontends/test_tensorflow/test_tf_functions.py:207:5: E131 continuation line unaligned for hanging indent ivy_tests/test_ivy/test_frontends/test_tensorflow/test_tf_functions.py:260:1: W391 blank line at end of file

Creative-Ataraxia · 2022-08-06T14:47:57Z

finally caught all the lint :)

Also, what's the reason for test-frontend-torch / run-nightly-tests (pull_request) to fail?

FAILED ivy/ivy_tests/test_ivy/test_frontends/test_torch/test_indexing_slicing_joining_mutating_ops.py::test_torch_reshape[cpu-ivy.functional.backends.torch-True-False-torch_call-torch]

Creative-Ataraxia · 2022-08-07T07:16:04Z

@juliagsy Hi, I've moved this PR to #2708, could we proceed from there? :)

juliagsy · 2022-08-07T07:48:22Z

Hello! To answer your questions:

yes, the inputs will be the 2 args only
dtype_and_values return dtype, values only, you can use np.asarray(values, dtype=dtype), then the helper function will process the array internally

there are error logs printed if you open up the workflow, the messages can be checked through there!

as for the PR, we go by assignee so your new assignee will review your PR soon!

thanks for contributing!

Creative-Ataraxia · 2022-08-09T01:24:29Z

Hello! To answer your questions:

yes, the inputs will be the 2 args only

dtype_and_values return dtype, values only, you can use np.asarray(values, dtype=dtype), then the helper function will process the array internally

there are error logs printed if you open up the workflow, the messages can be checked through there!

as for the PR, we go by assignee so your new assignee will review your PR soon!

thanks for contributing!

@juliagsy Hi! thanks for the answer. As far as test functions go, I've noticed there are some boilerplates, could you tell me their purpose so I can properly use them for future PRs?

as_variable_flags, with_out, native_array_flags, container_flags, instance_method, fw

juliagsy · 2022-08-09T03:04:06Z

oh right, I should've phrase it better, so the args for test_function will be a set of common parameters + function input args

As we can have array instances or container instances for the inputs, these parameters are used to indicate whether to test the function with: (all are booleans)
container_flags/instance_method - array or container inputs
native_array_flags - ivy array or native array inputs

Some other args:
fw - as we support different backends (aka conversions), we configured it internally to test for 4 backends

We have a decorator to generate data for these as they are common to all test functions, but don't worry about this as we are currently working on it! I'll share an example from our previous test so that you have a better idea on how they're used!

Example: https://github.com/unifyai/ivy/blob/94d07431e1beedd61a7140167600a93e96ca95eb/ivy_tests/test_ivy/test_frontends/test_jax/test_jax_lax_operators.py#L12

so the parameters up till the frontend argument will be the same or similar for all tests, and arguments following are differing according to the function

we also have documentations for further explanations!
https://lets-unify.ai/ivy/deep_dive/17_ivy_frontends_tests.html

hope these will help in clearing your doubts!

Creative-Ataraxia · 2022-08-10T15:06:36Z

@juliagsy thank you for the detailed reply, very helpful!

so for container_flags instance_method, when are we supposed to use them? the docs didn't mention them

juliagsy · 2022-08-11T13:51:50Z

They're for testing the backend functions with variables/instances of type ivy.Array or ivy.Container (eg. x.add(y) where x is of type ivy.Container or ivy.Array)

It doesn't apply to frontend functions as these functions' behaviours follow their native framework, that's probably the reason it's not mentioned in the docs!

Hope this answers your question!

Creative-Ataraxia · 2022-08-12T03:42:21Z

@juliagsy
I see! that makes sense.

also, when we write the frontend functions, the arguments should match exactly what the original framework has correct? like so:

def stack(tensors, dim=0, *, out=None):
torch.stack(tensors, dim=0, *, out=None):

and the return should match exactly what the backends has correct?

return ivy.stack(tensors, dim, out=out)
def stack(
    arrays: Union[
        Tuple[ivy.Array], List[ivy.Array], Tuple[ivy.NativeArray], List[ivy.NativeArray]
    ],
    axis: int = 0,
    *,
    out: Optional[ivy.Array] = None,
) -> ivy.Array:

juliagsy · 2022-08-12T03:48:28Z

yep, that's right!

Creative-Ataraxia · 2022-08-12T04:20:54Z

yep, that's right!

thanks for the clarification :)

Creative-Ataraxia added 2 commits August 4, 2022 22:09

Add reshape

4999cf6

to frontends.torch.indexing_slicing_joining_mutating_ops

Add test_torch_reshape

9f4b562

to test_frontends.test_indexing_slicing_joining_mutating_ops

github-actions bot assigned juliagsy Aug 4, 2022

juliagsy linked an issue Aug 5, 2022 that may be closed by this pull request

reshape #2636

Closed

juliagsy suggested changes Aug 5, 2022

View reviewed changes

Creative-Ataraxia added 2 commits August 6, 2022 10:12

removing other tasks currently in PR

ad6f35c

removing other tasks currently in PR

5b1a30f

Creative-Ataraxia requested a review from juliagsy August 6, 2022 02:14

juliagsy reviewed Aug 6, 2022

View reviewed changes

Creative-Ataraxia added 2 commits August 6, 2022 13:53

Update indexing_slicing_joining_mutating_ops.py

87052a9

use ivy.reshape which is the main alias

Creative-Ataraxia requested a review from juliagsy August 6, 2022 07:17

Creative-Ataraxia added 10 commits August 6, 2022 21:41

add copy argument

add5051

to match backends.torch.reshape's arguments

rehaul test_reshape

055cd71

lint error:

65f0ae8

'ivy/functional/frontends/torch/indexing_slicing_joining_mutating_ops.py:8:1: E305 expected 2 blank lines after class or function definition, found 1'

lint error:

d1ad41c

replace all ' = ' with '='

lint error:

edcd460

1. undefined name 'ivy_np' 2. line too long (90 > 88 characters)

lint errors

b08d9aa

Merge branch 'unifyai:master' into add_reshape_frontends.torch.indexi…

9eaf90f

…ng_slicing_joining_mutating_ops

Fix linting errors:

c6d2b55

1. 31:1: E303 too many blank lines (3) 2. 37:1: E302 expected 2 blank lines, found 1 3. 42:1: W391 blank line at end of file

lint

50d3e9e

Creative-Ataraxia closed this Aug 7, 2022

Creative-Ataraxia deleted the add_reshape_frontends.torch.indexing_slicing_joining_mutating_ops branch August 7, 2022 04:36

vedpatwardhan removed a link to an issue Aug 8, 2022

reshape #2636

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add reshape frontends.torch.indexing_slicing_joining_mutating_ops #2637

Add reshape frontends.torch.indexing_slicing_joining_mutating_ops #2637

Creative-Ataraxia commented Aug 4, 2022

juliagsy left a comment

Creative-Ataraxia commented Aug 6, 2022 •

edited

Loading

Creative-Ataraxia commented Aug 6, 2022

juliagsy commented Aug 6, 2022

juliagsy left a comment

juliagsy Aug 6, 2022

juliagsy Aug 6, 2022

juliagsy Aug 6, 2022

juliagsy Aug 6, 2022

Creative-Ataraxia Aug 6, 2022

Creative-Ataraxia commented Aug 6, 2022

Creative-Ataraxia commented Aug 6, 2022

Creative-Ataraxia commented Aug 7, 2022

juliagsy commented Aug 7, 2022

Creative-Ataraxia commented Aug 9, 2022

juliagsy commented Aug 9, 2022

Creative-Ataraxia commented Aug 10, 2022

juliagsy commented Aug 11, 2022

Creative-Ataraxia commented Aug 12, 2022

juliagsy commented Aug 12, 2022

Creative-Ataraxia commented Aug 12, 2022

Add reshape frontends.torch.indexing_slicing_joining_mutating_ops #2637

Add reshape frontends.torch.indexing_slicing_joining_mutating_ops #2637

Conversation

Creative-Ataraxia commented Aug 4, 2022

juliagsy left a comment

Choose a reason for hiding this comment

Creative-Ataraxia commented Aug 6, 2022 • edited Loading

Creative-Ataraxia commented Aug 6, 2022

juliagsy commented Aug 6, 2022

juliagsy left a comment

Choose a reason for hiding this comment

juliagsy Aug 6, 2022

Choose a reason for hiding this comment

juliagsy Aug 6, 2022

Choose a reason for hiding this comment

juliagsy Aug 6, 2022

Choose a reason for hiding this comment

juliagsy Aug 6, 2022

Choose a reason for hiding this comment

Creative-Ataraxia Aug 6, 2022

Choose a reason for hiding this comment

Creative-Ataraxia commented Aug 6, 2022

Creative-Ataraxia commented Aug 6, 2022

Creative-Ataraxia commented Aug 7, 2022

juliagsy commented Aug 7, 2022

Creative-Ataraxia commented Aug 9, 2022

juliagsy commented Aug 9, 2022

Creative-Ataraxia commented Aug 10, 2022

juliagsy commented Aug 11, 2022

Creative-Ataraxia commented Aug 12, 2022

juliagsy commented Aug 12, 2022

Creative-Ataraxia commented Aug 12, 2022

Creative-Ataraxia commented Aug 6, 2022 •

edited

Loading