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

[Good First Issue][TF FE]: Support AdjustHue operation for TensorFlow #24033

Closed
rkazants opened this issue Apr 15, 2024 · 12 comments · Fixed by #24769
Closed

[Good First Issue][TF FE]: Support AdjustHue operation for TensorFlow #24033

rkazants opened this issue Apr 15, 2024 · 12 comments · Fixed by #24769
Assignees
Labels
category: TF FE OpenVINO TensorFlow FrontEnd good first issue Good for newcomers no_stale Do not mark as stale
Milestone

Comments

@rkazants
Copy link
Contributor

rkazants commented Apr 15, 2024

Context

OpenVINO component responsible for support of TensorFlow models is called as TensorFlow Frontend (TF FE). TF FE converts a model represented in TensorFlow opset to a model in OpenVINO opset.

In order to infer TensorFlow models with AdjustHue operation by OpenVINO, TF FE needs to be extended with this operation support.

What needs to be done?

For AdjustHue operation support, you need to implement the corresponding loader into TF FE op directory and to register it into the dictionary of Loaders. One loader is responsible for conversion (or decomposition) of one type of TensorFlow operation.

Here is an example of loader implementation for TensorFlow Einsum operation:

OutputVector translate_einsum_op(const NodeContext& node) { 
     auto op_type = node.get_op_type(); 
     TENSORFLOW_OP_VALIDATION(node, op_type == "Einsum", "Internal error: incorrect usage of translate_einsum_op."); 
     auto equation = node.get_attribute<std::string>("equation"); 
  
     OutputVector inputs; 
     for (size_t input_ind = 0; input_ind < node.get_input_size(); ++input_ind) { 
         inputs.push_back(node.get_input(input_ind)); 
     } 
  
     auto einsum = make_shared<Einsum>(inputs, equation); 
     set_node_name(node.get_name(), einsum); 
     return {einsum}; 
 } 

In this example, translate_einsum_op converts TF Einsum into OV Einsum. NodeContext object passed into the loader packs all info about inputs and attributes of Einsum operation. The loader retrieves an attribute of the equation by using the NodeContext::get_attribute() method, prepares input vector, creates Einsum operation from OV opset and returns a vector of outputs.

Responsibility of a loader is to parse operation attributes, prepare inputs and express TF operation via OV operations sub-graph. Example for Einsum demonstrates the resulted sub-graph with one operation. In PR #19007 you can see operation decomposition into multiple node sub-graph.

Once you are done with implementation of the translator, you need to implement the corresponding layer tests test_tf_AdjustHue.py and put it into layer_tests/tensorflow_tests directory. Example how to run some layer test:

export TEST_DEVICE=CPU
cd openvino/tests/layer_tests/tensorflow_tests
pytest test_tf_Shape.py

Example Pull Requests

Resources

Contact points

  • @openvinotoolkit/openvino-tf-frontend-maintainers
  • @rkazants in GitHub
  • rkazants in Discord

Ticket

No response

@rkazants rkazants added good first issue Good for newcomers category: TF FE OpenVINO TensorFlow FrontEnd no_stale Do not mark as stale labels Apr 15, 2024
@psmaxwell
Copy link

#WLB#+ .take

Copy link
Contributor

Thank you for looking into this issue! Please let us know if you have any questions or require any help.

@rkazants
Copy link
Contributor Author

Hi @psmaxwell, any update on this task?

@rkazants
Copy link
Contributor Author

Hi @psmaxwell, I released this one ticket because you have another one. Please complete that one.

Best regards,
Roman

@oxkitsune
Copy link

.take

Copy link
Contributor

Thank you for looking into this issue! Please let us know if you have any questions or require any help.

@rkazants
Copy link
Contributor Author

rkazants commented May 2, 2024

Hi @oxkitsune, any update on this task?

@oxkitsune
Copy link

oxkitsune commented May 2, 2024

Hi @oxkitsune, any update on this task?

Yes! It took me a bit to get my environment set up on macos, but I got it working.

Working on this issue now!

@rkazants
Copy link
Contributor Author

Good reference how it was implemented for AdjustSaturation: #24511

@duydl
Copy link
Contributor

duydl commented May 29, 2024

.take

Copy link
Contributor

Thank you for looking into this issue! Please let us know if you have any questions or require any help.

@rkazants rkazants linked a pull request May 29, 2024 that will close this issue
ilya-lavrenov pushed a commit that referenced this issue Jun 2, 2024
### Details:
- Adapt from #24511 . Converting code repeated. Instead import from
there?
- Converting logic a little different from
https://github.com/tensorflow/tensorflow/blob/master/tensorflow/core/kernels/image/adjust_hue_op.cc
. Maybe less efficient?

### Tickets:
 - #24033 

Sorry, commit history is wrong, should have created branch from latest
upstream.

---------

Co-authored-by: Roman Kazantsev <roman.kazantsev@intel.com>
@rkazants
Copy link
Contributor Author

rkazants commented Jun 2, 2024

@duydl, congrats with another one merge. Let us go back to MatrixDiag op.

Best regards,
Roman

@mlukasze mlukasze added this to the 2024.3 milestone Jun 3, 2024
github-merge-queue bot pushed a commit that referenced this issue Jun 10, 2024
### Details:
- Using #24033, implemented and registered loader for HSVToRGB using
already existing logic.
 - Created unit test test_tf_HSVToRGB.py

### Tickets:
 - #24791 
 
Currently, my pytest is saying that for the unit test and for
test_tf_AdjustHue.py and test_tf_AdjustSaturation.py the loader is not
found.

---------

Co-authored-by: Roman Kazantsev <roman.kazantsev@intel.com>
alexandruenache1111 pushed a commit to alexandruenache1111/openvino that referenced this issue Jun 13, 2024
)

### Details:
- Using openvinotoolkit#24033, implemented and registered loader for HSVToRGB using
already existing logic.
 - Created unit test test_tf_HSVToRGB.py

### Tickets:
 - openvinotoolkit#24791 
 
Currently, my pytest is saying that for the unit test and for
test_tf_AdjustHue.py and test_tf_AdjustSaturation.py the loader is not
found.

---------

Co-authored-by: Roman Kazantsev <roman.kazantsev@intel.com>
allnes pushed a commit to allnes/openvino that referenced this issue Jun 27, 2024
### Details:
- Adapt from openvinotoolkit#24511 . Converting code repeated. Instead import from
there?
- Converting logic a little different from
https://github.com/tensorflow/tensorflow/blob/master/tensorflow/core/kernels/image/adjust_hue_op.cc
. Maybe less efficient?

### Tickets:
 - openvinotoolkit#24033 

Sorry, commit history is wrong, should have created branch from latest
upstream.

---------

Co-authored-by: Roman Kazantsev <roman.kazantsev@intel.com>
allnes pushed a commit to allnes/openvino that referenced this issue Jun 27, 2024
)

### Details:
- Using openvinotoolkit#24033, implemented and registered loader for HSVToRGB using
already existing logic.
 - Created unit test test_tf_HSVToRGB.py

### Tickets:
 - openvinotoolkit#24791 
 
Currently, my pytest is saying that for the unit test and for
test_tf_AdjustHue.py and test_tf_AdjustSaturation.py the loader is not
found.

---------

Co-authored-by: Roman Kazantsev <roman.kazantsev@intel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: TF FE OpenVINO TensorFlow FrontEnd good first issue Good for newcomers no_stale Do not mark as stale
Projects
Status: Closed
Development

Successfully merging a pull request may close this issue.

5 participants