ivy icon indicating copy to clipboard operation
ivy copied to clipboard

Imag for tf.raw_ops

Open Enigmafiend opened this issue 1 year ago • 10 comments

Close https://github.com/unifyai/ivy/issues/17595

Enigmafiend avatar Jun 22 '23 19:06 Enigmafiend

To streamline the review process, can you make a comment and write "add_frontend_checklist" (without double quotes). Our bots will edit your comment to a checklist you need to follow, please tick the points you have completed so I can review it

KareemMAX avatar Jun 23 '23 11:06 KareemMAX

If you are working on an open task, please edit the PR description to link to the issue you've created.

For more information, please check ToDo List Issues Guide.

Thank you :hugs:

ivy-leaves avatar Jun 23 '23 11:06 ivy-leaves

add_frontend_checklist

Enigmafiend avatar Jun 23 '23 14:06 Enigmafiend

removed a print statement and a supported dtype mislabel

Enigmafiend avatar Jun 24 '23 16:06 Enigmafiend

@Enigmafiend seems like our automation didn't pickup the comment because you added a new line 😅 would you mind making another comment without that newline? Just "add_frontend_checklist". After that please fill the checklist accordingly

KareemMAX avatar Jun 26 '23 09:06 KareemMAX

ivy-gardener :white_check_mark: Ivy gardener has formatted your code. If changes are requested, don't forget to pull your fork.

KareemMAX avatar Jun 26 '23 10:06 KareemMAX

@Enigmafiend following up on the map_raw_ops_alias, sadly your function doesn't qualify as an alias in that case. So you must rewrite it as a composite function so we can merge it.

Once you finish with this change please let me know by re-requesting review on github

KareemMAX avatar Jun 26 '23 10:06 KareemMAX

ivy-gardener :white_check_mark: Ivy gardener has formatted your code. If changes are requested, don't forget to pull your fork.

Enigmafiend avatar Jun 26 '23 17:06 Enigmafiend

Frontend Task Checklist

IMPORTANT NOTICE 🚨:

The Ivy Docs represent the ground truth for the task descriptions and this checklist should only be used as a supplementary item to aid with the review process.

LEGEND 🗺:

  • ❌ : Check item is not completed.
  • ✅ : Check item is ready for review.
  • 🆘 : Stuck/Doubting implementation (PR author should add comments explaining why).
  • ⏩ : Check is not applicable to function (skip).
  • 🆗 : Check item is implemented and does not require any edits.

CHECKS 📑:

    • [x] 🆗: The function/method definition is not missing any of the original arguments.
    • [x] ⏩: In case the function/method to be implemented is an alias of an existing function/method:
        • [x] ⏩: It is being declared as such by setting fun1 = fun2, rather than being re-implemented from scratch.
        • [x] ⏩: The alias is added to the existing function/method's test in the aliases parameter of handle_frontend_test/handle_frontend_method.
    • [x] 🆗: The naming of the function/method and its arguments exactly matches the original.
    • [x] 🆗: No defined argument is being ignored in the function/method's implementation.
    • [x] ⏩: In special cases where an argument's implementation should be pending due to an incomplete superset of an ivy function:
        • [x] ⏩: A descriptive comment has been left under the Implement superset behavior ToDo list in https://github.com/unifyai/ivy/issues/6406.
        • [x] ⏩: A ToDo comment has been added prompting to pass the frontend argument to the ivy function whose behavior is to be extended.
    • [x] 🆗: In case a frontend function is being added:
        • [x] 🆗: It is a composition of ivy functions.
        • [x] ⏩: In case the needed composition is long (using numerous ivy functions), a Missing Function Suggestion issue has been opened to suggest a new ivy function should be added to shorten the frontend implementation.
        • [x] 🆗: @to_ivy_arrays_and_back has been added to the function.
    • [x] ⏩: In case a frontend method is being added:
        • [x] ⏩: It is composed of existing frontend functions or methods.
        • [x] ⏩: If a required frontend function has not yet been added, the method may be implemented as a composition of ivy functions, making sure that:
          • [x] ⏩: @to_ivy_arrays_and_back has been added to the method.
          • [x] ⏩: A ToDo comment has been made prompting to remove the decorator and update the implementation as soon as the missing function has been added.
    • [x] 🆗: The function/method's test has been added (except in the alias case mentioned in <2>):
        • [x] 🆗: All supported arguments are being generated in handle_frontend_test/handle_frontend_method and passed to test_frontend_function/test_frontend_method.
        • [x] 🆗: The argument generation covers all possible supported values. Array sizes, dimensions, and axes adhere to the full supported set of the original function/method.
        • [ ] ❌: The available_dtypes parameter passed to the helper generating the function/method's input array is set to helpers.get_dtypes("valid"). If there are unsupported dtypes that cause the test to fail, they should be handled by adding @with_supported_dtypes/@with_unsupported_dtype to the function/method.
    • [x] 🆗: The PR is not introducing any test failures.
        • [x] 🆗: The lint checks are passing.
        • [x] 🆗: The implemented test is passing for all backends.
    • [x] 🆗: The PR closes a Sub Task issue linked to one of the open frontend ToDo lists.
    • [x] 🆗: The function/method and its test have been added to the correct .py files corresponding to the addressed ToDo list.
    • [x] 🆗: The PR only contains changes relevant to the addressed subtask.

Enigmafiend avatar Jun 26 '23 17:06 Enigmafiend

ivy-gardener :white_check_mark: Ivy gardener has formatted your code. If changes are requested, don't forget to pull your fork.

Enigmafiend avatar Jun 30 '23 20:06 Enigmafiend

Another point to note that I missed to mention when we talked, is that the Tout coupling makes sense as a complex 64 is practically 2 Float 32 Elements combined into one (Hence the increased memory as 32 bits for the Real element and 32 for the Imaginary) therefore trying to assign it to a Tout of float64 is an invalid means of memory allocation and not accepted by the cpu setup

Enigmafiend avatar Jun 30 '23 21:06 Enigmafiend

ivy-gardener :white_check_mark: Ivy gardener has formatted your code. If changes are requested, don't forget to pull your fork.

Enigmafiend avatar Jul 04 '23 18:07 Enigmafiend