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

fix: Reformatted gather to use torch backend for its backend-specific implementation. Updated docstrings. #26998

Closed
wants to merge 8 commits into from

Conversation

Kacper-W-Kozdon
Copy link
Contributor

@Kacper-W-Kozdon Kacper-W-Kozdon commented Oct 13, 2023

PR Description

fix #26498 Edited gather to use torch methods in backend. Updated docstrings.

Related Issue

Closes #26498

Checklist

  • [⏩] Did you add a function?
  • [⏩] Did you add the tests?
  • [❌] Did you run your tests and are your tests passing?
  • [✅] Did pre-commit not fail on any check?
  • [✅] Did you follow the steps we provided?

Socials

fix: Updated docstring for gather.
fix(backend): Edited gather to incorporate backend-specific torch methods.
fix(backend): Edited gather to incorporate backend-specific torch methods.
fix(backend): Edited gather to use torch methods.
fix: Edited fault docstrings.
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

PR Compliance Checks Passed!

@Kacper-W-Kozdon
Copy link
Contributor Author

Kacper-W-Kozdon commented Oct 13, 2023

Reformatting 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 already implemented and does not require any edits.

CHECKS 📑:

    • ❌: Make sure that the aforementioned methods are added into the correct category-specific parent class, such as ivy.ArrayWithElementwise, ivy.ContainerWithManipulation etc.
    • ❌: Add the correct Docstrings to every function and its relevant methods, including those you did not implement yourself. The following should be added:
        • ❌: The function's Array API standard description in ivy/functional/Reformatting_general.py. If the function is not part of the Array API standard then a description of similar style should be added to the same file.
          The following modifications should be made to the description:
          • ✅: Remove type definitions in the Parameters and Returns sections.
          • 🆗: Add out to the Parameters section if function accepts an out argument.
          • 🆗: Replace out with ret in the Returns section.
    • 🆗: Add thorough Docstring Examples for every function and its relevant methods and ensure they pass the docstring tests.

      Functional Examples in ivy/functional/Reformatting_general.py.

        • ❌: Cover all possible variants for each of the arguments independently (not combinatorily).
        • ❌: Vary the values and input shapes considerably between examples.
        • 🆗: Start out simple and get more complex with each example.
        • 🆗: Show an example with:
          • 🆗: out unused.
          • 🆗: out used to update a new array y.
          • 🆗: out used to inplace update the input array x (if x has the same dtype and shape as the return).
        • ❌: If broadcasting is relevant for the function, then show examples which highlight this.

      Nestable Function Examples in ivy/functional/Reformatting_general.py.
      Only if the function supports nestable operations.

        • 🆗: Add an example that passes in an ivy.Container instance in place of one of the arguments.
        • 🆗: Add an example passes in ivy.Container instances for multiple arguments.

      Container Static Method Examples in ivy/container/Reformatting_general.py.

        • ❌: The example from point (6.f) should be replicated, but added to the ivy.Container static method docstring in with ivy.<func_name> replaced with ivy.Container.static_<func_name> in the example.
        • ❌: The example from point (6.g) should be replicated, but added to the ivy.Container static method docstring, with ivy.<func_name> replaced with ivy.Container.static_<func_name> in the example.

      Array Instance Method Example in ivy/array/Reformatting_general.py.

        • ❌: Call this instance method of the ivy.Array class.

      Container Instance Method Example in ivy/container/Reformatting_general.py.

        • ❌: Call this instance method of the ivy.Container class.

      Array Operator Examples in ivy/array/array.py.

        • ⏩: Call the operator on two ivy.Array instances.
        • ⏩: Call the operator with an ivy.Array instance on the left and ivy.Container on the right.

      Array Reverse Operator Example in ivy/array/array.py.

        • ⏩: Call the operator with a Number on the left and an ivy.Array instance on the right.

      Container Operator Examples in ivy/container/container.py.

        • ⏩: Call the operator on two ivy.Container instances containing Number instances at the leaves.
        • ⏩: Call the operator on two ivy.Container instances containing ivy.Array instances at the leaves.
        • ⏩: Call the operator with an ivy.Container instance on the left and ivy.Array on the right.

      Container Reverse Operator Example in ivy/container/container.py.

        • ⏩: Following example in the ivy.Container.__radd__ docstring, with the operator called with a Number on the left and an ivy.Container instance on the right.

      Tests

        • ❌: Docstring examples tests passing.
        • ✅: Lint checks passing.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Congrats on making your first Pull Request and thanks for supporting Ivy! 🎉
Join the conversation in our Discord.

For every PR opened, we run unit tests and comment the results in the PR to ensure the functionality remains intact.
Please make sure they are passing. 💪

If you want to check them from the action runners, you can open the display_test_results job. 👀
It contains the following two sections:

  • Combined Test Results: This shows the results of all the ivy tests that ran on the PR. ✔️
  • New Failures Introduced: This lists the tests that fail on this PR.

Keep in mind that we will assign an engineer for this task and they will look at it based on the workload that they have, kindly be patient 😄.

@Kacper-W-Kozdon Kacper-W-Kozdon changed the title - [ ] #26498 fix: Reformatted gather to use torch backend for its backend-specific implementation. Updated docstrings. Oct 13, 2023
@github-actions
Copy link
Contributor

Thank you for this PR, here is the CI results:


This pull request does not result in any additional test failures. Congratulations!

@Kacper-W-Kozdon Kacper-W-Kozdon marked this pull request as draft October 18, 2023 18:14
@Sam-Armstrong
Copy link
Contributor

@Kacper-W-Kozdon I see you have a separate PR for this here #27071, so I'm assuming this can be closed?

@Kacper-W-Kozdon
Copy link
Contributor Author

@Kacper-W-Kozdon I see you have a separate PR for this here #27071, so I'm assuming this can be closed?

Yes, this one can be closed, sorry for the confusion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

gather
4 participants