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

[PT FE] Add aten::atan2 #23003

Closed
wants to merge 51 commits into from
Closed

Conversation

rghvsh
Copy link
Contributor

@rghvsh rghvsh commented Feb 22, 2024

Details:

  • Adds aten::atan2 operator

Tickets:

@rghvsh rghvsh requested a review from a team as a code owner February 22, 2024 02:44
@github-actions github-actions bot added the category: PyTorch FE OpenVINO PyTorch Frontend label Feb 22, 2024
@rghvsh rghvsh changed the title [PT FE] Add aten::linspace [PT FE] Add aten::atan2 Feb 22, 2024
Copy link
Contributor

@mvafin mvafin left a comment

Choose a reason for hiding this comment

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

Please check that code builds before submitting

src/frontends/pytorch/src/op/atan2.cpp Outdated Show resolved Hide resolved
@rghvsh
Copy link
Contributor Author

rghvsh commented Feb 22, 2024

Hey @mvafin! I wrote this code for tffe this was a first draft will work on this

@rghvsh rghvsh requested a review from mvafin February 22, 2024 14:09
@ilya-lavrenov ilya-lavrenov added the ExternalPR External contributor label Feb 23, 2024
@rghvsh
Copy link
Contributor Author

rghvsh commented Mar 3, 2024

Hi! @mvafin made some changes is this better?

Thanks
Raghav

Copy link
Contributor

@mvafin mvafin left a comment

Choose a reason for hiding this comment

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

Also

src/frontends/pytorch/src/op/atan2.cpp Outdated Show resolved Hide resolved
src/frontends/pytorch/src/op/atan2.cpp Outdated Show resolved Hide resolved
fix build issues and code style
add different types for x and y
@rghvsh
Copy link
Contributor Author

rghvsh commented Mar 26, 2024

Hi! @mvafin @mmikolajcz I think i've fixed build issues and added test for different types in x and y. I had some questions:
As there is optional_out decorator I don't need to use mutate_input or apply the if block and return result straight away?
How to fix the get inputs with promoted types?

PS: the CI tests are failing with no error logs,
Thanks,
Raghav

src/frontends/pytorch/src/op/atan2.cpp Show resolved Hide resolved
src/frontends/pytorch/src/op/atan2.cpp Outdated Show resolved Hide resolved
src/frontends/pytorch/src/op/atan2.cpp Outdated Show resolved Hide resolved
src/frontends/pytorch/src/op/atan2.cpp Outdated Show resolved Hide resolved
src/frontends/pytorch/src/op/atan2.cpp Outdated Show resolved Hide resolved
src/frontends/pytorch/src/op/atan2.cpp Outdated Show resolved Hide resolved
src/frontends/pytorch/src/op/atan2.cpp Outdated Show resolved Hide resolved
src/frontends/pytorch/src/op/atan2.cpp Outdated Show resolved Hide resolved
tests/layer_tests/pytorch_tests/test_atan2.py Outdated Show resolved Hide resolved
src/frontends/pytorch/src/op/atan2.cpp Outdated Show resolved Hide resolved
src/frontends/pytorch/src/op/atan2.cpp Show resolved Hide resolved
rghvsh and others added 2 commits March 26, 2024 21:34
Co-authored-by: Mateusz Mikolajczyk <mateusz.mikolajczyk@intel.com>
@rghvsh
Copy link
Contributor Author

rghvsh commented Mar 26, 2024

Hi @mvafin @mmikolajcz! Made the changes. Last time for the CI I guess the cmake did not configure properly.

src/frontends/pytorch/src/op/atan2.cpp Outdated Show resolved Hide resolved
src/frontends/pytorch/src/op/atan2.cpp Outdated Show resolved Hide resolved
@rghvsh
Copy link
Contributor Author

rghvsh commented Mar 26, 2024

Hi @mvafin @mmikolajcz! Made the changes

@mlukasze
Copy link
Contributor

build_jenkins

@mvafin
Copy link
Contributor

mvafin commented Mar 27, 2024

Tests fail with

    @pytest.mark.parametrize("use_out", [False, True])
    def test_atan2_with_out(self, dtype1, dtype2, use_out, y, x, ie_device, precision, ir_version):
        self._test(
>           *self.create_model(dtype=dtype, use_out=use_out),
            ie_device,
            precision,
            ir_version,
            kwargs_to_prepare_input={"y": y, "x": x}
        )
E       NameError: name 'dtype' is not defined

@mvafin
Copy link
Contributor

mvafin commented Apr 2, 2024

Still same error:

    def test_atan2_with_out(self, dtype1, dtype2, use_out, y, x, ie_device, precision, ir_version):
        self._test(
>           *self.create_model(dtype=dtype1, use_out=use_out),
            ie_device,
            precision,
            ir_version,
            kwargs_to_prepare_input={"y": y, "x": x}
        )
E       TypeError: TestAtan2.create_model() got an unexpected keyword argument 'dtype'

Copy link
Contributor

This PR will be closed in a week because of 2 weeks of no activity.

@github-actions github-actions bot added the Stale label Apr 26, 2024
@mlukasze
Copy link
Contributor

hey @rghvsh will you find a time to address Maxim's requests?

@github-actions github-actions bot removed the Stale label Apr 27, 2024
Copy link
Contributor

This PR will be closed in a week because of 2 weeks of no activity.

@github-actions github-actions bot added the Stale label May 11, 2024
Copy link
Contributor

This PR was closed because it has been stalled for 2 week with no activity.

@github-actions github-actions bot closed this May 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: PyTorch FE OpenVINO PyTorch Frontend ExternalPR External contributor Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Good First Issue]: Support aten::atan2 for pytorch models
5 participants