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

[JAX]: Support jax.lax.select_n operation for JAX #28025

Merged
merged 14 commits into from
Dec 31, 2024

Conversation

11happy
Copy link
Contributor

@11happy 11happy commented Dec 11, 2024

Overview:
This pull request fixes #26570.

Testing:

  • Tested the updated code.
  • Verified that other functionalities remain unaffected.
    Screenshot from 2024-12-12 00-09-17

Dependencies:

  • No dependencies on other pull requests.

CC:

@11happy 11happy requested review from a team as code owners December 11, 2024 18:40
@github-actions github-actions bot added category: TF FE OpenVINO TensorFlow FrontEnd category: JAX FE OpenVINO JAX FrontEnd labels Dec 11, 2024
@sys-openvino-ci sys-openvino-ci added the ExternalPR External contributor label Dec 11, 2024
@rkazants
Copy link
Contributor

build_jenkins

@11happy 11happy requested a review from rkazants December 13, 2024 04:43
@11happy
Copy link
Contributor Author

11happy commented Dec 14, 2024

humble ping! @rkazants

@11happy
Copy link
Contributor Author

11happy commented Dec 16, 2024

Hello @rkazants, please let me know if there's anything needed from my end.

@11happy 11happy requested a review from rkazants December 19, 2024 17:28
@rkazants
Copy link
Contributor

build_jenkins

@rkazants rkazants added this to the 2025.0 milestone Dec 23, 2024
@rkazants
Copy link
Contributor

build_jenkins

@rkazants
Copy link
Contributor

@11happy, please pay attention to failing CI jobs (code-style, build, etc.), it needs to be fixed.


@pytest.mark.parametrize("input_shape", [[],[1],[2,3],[4,5,6],[7,8,9,10]])
@pytest.mark.parametrize("input_type", [np.int32, np.int64, bool])
@pytest.mark.parametrize("case_num", [1,2,3,4,5,6,7,8,9,10])
Copy link
Contributor

Choose a reason for hiding this comment

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

let use case_num starting from 2. Value 1 is impractical.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@rkazants
Copy link
Contributor

build_jenkins

@rkazants
Copy link
Contributor

This time I ran clang-format on my code what else I can do to ensure it passes CI jobs?. Thank you

Let CI complete and check failing jobs in GHA. You may see build issues that need to be fixed.

Signed-off-by: 11happy <[email protected]>
@rkazants
Copy link
Contributor

build_jenkins

@rkazants
Copy link
Contributor

rkazants commented Dec 30, 2024

@11happy, looks good. Let us see CI results. If it is fine, I will merge it.

Best regards,
Roman

@rkazants rkazants self-assigned this Dec 30, 2024
@rkazants
Copy link
Contributor

rkazants commented Dec 30, 2024

@11happy, please fix CI errors. Check GHA jobs. Example:

src/frontends/jax/src/CMakeFiles/openvino_jax_frontend.dir/op/select_n.cpp.o -c /__w/openvino/openvino/openvino/src/frontends/jax/src/op/select_n.cpp
/__w/openvino/openvino/openvino/src/frontends/jax/src/op/select_n.cpp:5:50: error: extra tokens at end of #include directive [-Werror,-Wextra-tokens]
#include "openvino/frontend/jax/node_context.hpp";
                                                 ^
                                                 //
/__w/openvino/openvino/openvino/src/frontends/jax/src/op/select_n.cpp:11:21: error: extra tokens at end of #include directive [-Werror,-Wextra-tokens]
#include "utils.hpp";
                    ^
                    //
2 errors generated.

@11happy
Copy link
Contributor Author

11happy commented Dec 30, 2024

@11happy, please fix CI errors. Check GHA jobs. Example:

src/frontends/jax/src/CMakeFiles/openvino_jax_frontend.dir/op/select_n.cpp.o -c /__w/openvino/openvino/openvino/src/frontends/jax/src/op/select_n.cpp
/__w/openvino/openvino/openvino/src/frontends/jax/src/op/select_n.cpp:5:50: error: extra tokens at end of #include directive [-Werror,-Wextra-tokens]
#include "openvino/frontend/jax/node_context.hpp";
                                                 ^
                                                 //
/__w/openvino/openvino/openvino/src/frontends/jax/src/op/select_n.cpp:11:21: error: extra tokens at end of #include directive [-Werror,-Wextra-tokens]
#include "utils.hpp";
                    ^
                    //
2 errors generated.

I have removed these extra semi colons.

@rkazants
Copy link
Contributor

build_jenkins

@11happy
Copy link
Contributor Author

11happy commented Dec 31, 2024

I saw this gha fail:

numpy.int64'> - input_shape:[2, 3] ] - AssertionError: number of outputs not equal, 1 != 2
FAILED install/tests/layer_tests/jax_tests/test_select_n.py::TestSelectN::test_select_n[ ie_device:CPU - precision:FP16 - case_num:3 - input_type:<class 'numpy.int64'> - input_shape:[4, 5, 6] ] - AssertionError: number of outputs not equal, 1 != 2
FAILED install/tests/layer_tests/jax_tests/test_select_n.py::TestSelectN::test_select_n[ ie_device:CPU - precision:FP16 - case_num:3 - input_type:<class 'numpy.int64'> - input_shape:[7, 8, 9, 10] ] - AssertionError: number of outputs not equal, 1 != 2
=============== 10 failed, 1500 passed, 1510 warnings in 37.14s ================

How can I resolve this?

@rkazants
Copy link
Contributor

build_jenkins

@rkazants
Copy link
Contributor

How can I resolve this?

It looks a problem of JAX utils itself that helps to convert JAX model to TF model. I switched off a path via TF FE that is deprecated anyway.

Best regards,
Roman

@rkazants
Copy link
Contributor

build_jenkins

1 similar comment
@rkazants
Copy link
Contributor

build_jenkins

@rkazants rkazants enabled auto-merge December 31, 2024 12:23
@rkazants rkazants added this pull request to the merge queue Dec 31, 2024
Merged via the queue into openvinotoolkit:master with commit 362f073 Dec 31, 2024
182 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: JAX FE OpenVINO JAX FrontEnd category: TF FE OpenVINO TensorFlow FrontEnd ExternalPR External contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Good First Issue][JAX FE]: Support jax.lax.select_n operation for JAX
3 participants