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

[TF FE] Support AdjustSaturation operation #24511

Merged
merged 34 commits into from
May 29, 2024

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 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

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

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

@duydl
Copy link
Contributor Author

duydl commented May 28, 2024

Thanks for the review. All except changing Gather to Select have been addressed, reason I commented under the relevant review.

Copy link
Contributor

@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.

last comments, The other code looks good to me

@rkazants
Copy link
Contributor

build_jenkins

@rkazants rkazants added this to the 2024.3 milestone May 28, 2024
@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

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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