Skip to content

[TF FE] Support MatrixDiagV3 operation #24498

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

Closed
wants to merge 11 commits into from

Conversation

duydl
Copy link
Contributor

@duydl duydl commented May 14, 2024

@github-actions github-actions bot added category: docs OpenVINO documentation category: TF FE OpenVINO TensorFlow FrontEnd category: TFL FE OpenVINO TensorFlow Lite FrontEnd no-match-files labels May 14, 2024
@sys-openvino-ci sys-openvino-ci added the ExternalPR External contributor label May 14, 2024
@rkazants rkazants requested a review from popovaan May 14, 2024 05:03
Copy link
Member

@rkazants rkazants left a comment

Choose a reason for hiding this comment

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

Good start, keep this way!

@rkazants
Copy link
Member

build_jenkins

@rkazants
Copy link
Member

Hi @duydl, any progress on the next steps? Please finalize PR.

@duydl
Copy link
Contributor Author

duydl commented May 23, 2024

@rkazants Hi, I have some problem with retrieving attribute from tf graph: attribute always get the fallback value.

    auto k = node.get_attribute<int64_t>("k", 0); // get an int instead of dynamic size node
    auto num_rows = node.get_attribute<int64_t>("num_rows", -1); 
    auto num_cols = node.get_attribute<int64_t>("num_cols", -1); 
    auto padding_value_ = node.get_attribute<int64_t>("padding_value", 0);

I check other ops using this but couldn't see the difference in how attribute passed into tf graph in the pytest file.

@duydl duydl force-pushed the tf_fe/matrix_diag_v3 branch from c70f4b5 to 48bf0b5 Compare May 23, 2024 03:38
@rkazants rkazants marked this pull request as ready for review May 23, 2024 08:35
@rkazants rkazants requested review from a team as code owners May 23, 2024 08:35
@rkazants rkazants requested review from kblaszczak-intel and removed request for a team May 23, 2024 08:35
@rkazants
Copy link
Member

@rkazants Hi, I have some problem with retrieving attribute from tf graph: attribute always get the fallback value.

I think these are not attributes and should be treat as input tensors and accessed with node.get_input(i). You just need to check get_input_size that will tell how many inputs and what are inputs are not passed and we need to use default value for this.

@duydl
Copy link
Contributor Author

duydl commented May 23, 2024

@rkazants Hi, I have some problem with retrieving attribute from tf graph: attribute always get the fallback value.

I think these are not attributes and should be treat as input tensors and accessed with node.get_input(i). You just need to check get_input_size that will tell how many inputs and what are inputs are not passed and we need to use default value for this.

So params could only be either attribute or input and it depend on tf internal? I originally use get_input to get k but get this error:

E       FrontEnd API failed with OpConversionFailure:
E       [TensorFlow Frontend] Internal error, conversion is failed for MatrixDiagV3 operation with a message:
E       Exception from src/core/src/partial_shape.cpp:268:
E       to_shape was called on a dynamic shape.

from this snippet.

    auto axes = make_shared<v0::Constant>(element::i64, Shape{1}, std::vector<int64_t>{0});
    auto k_unsqueezed = make_shared<v0::Unsqueeze>(k_, axes);
    auto updated_shape = make_shared<v3::ScatterElementsUpdate>(
      zero_padded_diag_shape, // data input
      make_shared<v0::Constant>(element::i64, Shape{1}, std::vector<int64_t>{-1}), // indices
      k_unsqueezed, // updates 
      make_shared<v0::Constant>(element::i64, Shape{}, std::vector<int64_t>{0}) // axis 
    );

I expected it is because k is from node input so change to attribute instead. But as it is not possible, do you have any insight?

Edit: ie, openVINO ops nodes need to be config with value of k (and some other params of the tf ops) but seem like it is not possible to access the value as it is considered dynamic.

@ilya-lavrenov ilya-lavrenov requested a review from rkazants June 16, 2024 21:31
Copy link
Contributor

github-actions bot commented Jul 1, 2024

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

@github-actions github-actions bot added the Stale label Jul 1, 2024
@duydl duydl closed this Jul 6, 2024
@duydl
Copy link
Contributor Author

duydl commented Jul 6, 2024

Hi, I currently do not have time to try other approaches. I will leave it to others and may reopen the pr to retry if available later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: docs OpenVINO documentation category: TF FE OpenVINO TensorFlow FrontEnd category: TFL FE OpenVINO TensorFlow Lite FrontEnd ExternalPR External contributor Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Good First Issue][TF FE]: Support MatrixDiagV3 operation for TensorFlow
3 participants