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.

Sign up for GitHub

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

Jump to bottom

[TF FE] Support AdjustSaturation operation #24511

Merged
merged 34 commits into from
May 29, 2024
Merged

[TF FE] Support AdjustSaturation operation #24511

merged 34 commits into from
May 29, 2024

Conversation

duydl
Copy link
Contributor

@duydl duydl commented May 14, 2024 •
edited by rkazants
Loading

@github-actions github-actions bot added category: docs OpenVINO documentation category: TF FE OpenVINO TensorFlow FrontEnd category: TFL FE OpenVINO TensorFlow Lite FrontEnd labels May 14, 2024
@sys-openvino-ci sys-openvino-ci added the ExternalPR External contributor label May 14, 2024
@duydl
Copy link
Contributor Author

duydl commented May 14, 2024 •
edited
Loading

Steps for adjust saturation in adjust_saturation_op.cc:

  • Convert RGB to HSV
  • Scale the saturation component of HSV, easily done with Multiply
  • Convert HSV back to RGB

tf have the custom conversion implementations in adjust_saturation_op.cc. The major work would likely be adapting it to openVINO ops.

@rkazants
Copy link
Contributor

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

@duydl
Copy link
Contributor Author

duydl commented May 21, 2024 •
edited
Loading

Sorry for the delay. I was at this stage when started travelling last week. There is assert error like this:

AssertionError: Comparing with Framework failed: ie_res={'AdjustSaturation:0': array([[[[0.07344759, 0.05478147, 0.05478147],
E                [0.48940325, 0.30517435, 0.30517435]]]], dtype=float32)}; framework_res={'AdjustSaturation:0': array([[[[0.07344764, 0.50295335, 0.05478147],
E                [0.48940322, 0.8983458 , 0.3051743 ]]]], dtype=float32)}.

So I expect there is a small error in hsv_to_rgb but couldn't determine it yet.

@duydl duydl force-pushed the tf_fe/adjust_saturation_op branch from ec9c917 to d9fea6c Compare May 21, 2024 19:23
@duydl
Copy link
Contributor Author

duydl commented May 22, 2024

@rkazants Hi, I think it is good enough for review now.

@rkazants rkazants marked this pull request as ready for review May 23, 2024 07:42
@rkazants rkazants requested review from a team as code owners May 23, 2024 07:42
@rkazants rkazants requested review from kblaszczak-intel and removed request for a team May 23, 2024 07:42
@rkazants
Copy link
Contributor

build_jenkins

@rkazants
Copy link
Contributor

build_jenkins

@rkazants rkazants added this to the 2024.3 milestone May 28, 2024
@rkazants rkazants linked an issue May 28, 2024 that may be closed by this pull request
[Good First Issue][TF FE]: Support AdjustSaturation operation for TensorFlow #24034
Closed
@duydl
Copy link
Contributor Author

duydl commented May 28, 2024

Addressed your review. Thanks for the guide.

@rkazants
Copy link
Contributor

build_jenkins

@rkazants
Copy link
Contributor

@duydl, please fix code-style: https://github.com/openvinotoolkit/openvino/actions/runs/9264167264/job/25487017384?pr=24511

please apply clang formatting

@rkazants
Copy link
Contributor

build_jenkins

@rkazants rkazants self-assigned this May 28, 2024
@rkazants rkazants enabled auto-merge May 28, 2024 09:10
@rkazants
Copy link
Contributor

build_jenkins

@rkazants rkazants added this pull request to the merge queue May 29, 2024
Merged via the queue into openvinotoolkit:master with commit 3e3bea8 May 29, 2024
116 checks passed
@rkazants rkazants mentioned this pull request May 29, 2024
Closed
@duydl duydl mentioned this pull request May 29, 2024
Merged
This was referenced May 31, 2024
Closed
Open
ilya-lavrenov pushed a commit that referenced this pull request Jun 2, 2024
@duydl @rkazants
[TF FE] Support AdjustHue operation (#24769)
5ea0e96 
### 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>
allnes pushed a commit to allnes/openvino that referenced this pull request Jun 27, 2024
@duydl @rkazants @allnes
[TF FE] Support AdjustSaturation operation (openvinotoolkit#24511)
a818c3c 
### Details:
- Parse logic from
https://github.com/tensorflow/tensorflow/blob/master/tensorflow/core/kernels/image/adjust_saturation_op.cc
to openVINO ops

### Tickets:
 - openvinotoolkit#24034

---------

Co-authored-by: Roman Kazantsev <roman.kazantsev@intel.com>
allnes pushed a commit to allnes/openvino that referenced this pull request Jun 27, 2024
@duydl @rkazants @allnes
[TF FE] Support AdjustHue operation (openvinotoolkit#24769)
b72f87d 
### 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rkazants rkazants rkazants approved these changes

@kblaszczak-intel kblaszczak-intel Awaiting requested review from kblaszczak-intel kblaszczak-intel is a code owner automatically assigned from openvinotoolkit/openvino-docs-maintainers

Assignees

@rkazants rkazants

Labels
category: docs OpenVINO documentation category: TF FE OpenVINO TensorFlow FrontEnd category: TFL FE OpenVINO TensorFlow Lite FrontEnd ExternalPR External contributor
Projects
None yet
Milestone
2024.3
Development

Successfully merging this pull request may close these issues.

[Good First Issue][TF FE]: Support AdjustSaturation operation for TensorFlow
3 participants
@duydl @rkazants @sys-openvino-ci