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

BUGFIX: #28686

Merged
merged 1 commit into from May 23, 2019
Merged

BUGFIX: #28686

merged 1 commit into from May 23, 2019

Conversation

ThisIsIsaac
Copy link
Contributor

Fixed the bug of the kernel not fully processing all the items when the batch * height * width > number of threads spawned by adding a layer of for-loop

two spaces for minor performance increase:

  1. instead of taking hue_delta as a global memory, take in the value inside hue_delta in order to eliminate unnecessary global memory read
  2. make the copying performed in this conditional: if (!AdjustHue && !AdjustSaturation && !AdjustV) access global memory with coalesced accesses

@ThisIsIsaac
BUGFIX:
c73a146 
Fixed the bug of the kernel not fully processing all the items when the batch * height * width > number of threads spawned by adding a layer of for-loop

**two spaces for minor performance increase:**
1. instead of taking `hue_delta` as a global memory, take in the value inside `hue_delta` in order to eliminate unnecessary global memory read
2. make the copying performed in this conditional: `if (!AdjustHue && !AdjustSaturation && !AdjustV)` access global memory with coalesced accesses
@tensorflow-bot tensorflow-bot bot added the size:M CL Change Size: Medium label May 14, 2019
@ThisIsIsaac
Copy link
Contributor Author

@chsigg
a small bugfix of adjust_hsv_nhwc kernel ( more details in the commit note )

@gbaned gbaned self-assigned this May 14, 2019
@gbaned gbaned added this to Assigned Reviewer in PR Queue via automation May 14, 2019
@gbaned gbaned requested a review from chsigg May 14, 2019 02:48
@gbaned gbaned added the prtype:bugfix PR to fix a bug label May 14, 2019
@ThisIsIsaac ThisIsIsaac mentioned this pull request May 15, 2019
Closed
chsigg
chsigg approved these changes May 16, 2019
View reviewed changes
Copy link
Contributor

@chsigg chsigg left a comment

Choose a reason for hiding this comment

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

Thanks for the fix Isaac!

PR Queue automation moved this from Assigned Reviewer to Approved by Reviewer May 16, 2019
@tensorflow-bot tensorflow-bot bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels May 16, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label May 16, 2019
@ThisIsIsaac
Copy link
Contributor Author

@tensorflow-bot
run test again

@ThisIsIsaac
Copy link
Contributor Author

@chsigg
Two Windows Bazel builds failed. Could you help me out?

The error output is:

ERROR: T:/src/github/tensorflow/tensorflow/lite/python/interpreter_wrapper/BUILD:59:1: Linking of rule '//tensorflow/lite/python/interpreter_wrapper:_tensorflow_wrap_interpreter_wrapper.so' failed (Exit 1000): link.exe failed: error executing command

@tensorflow-copybara tensorflow-copybara merged commit c73a146 into tensorflow:master May 23, 2019
PR Queue automation moved this from Approved by Reviewer to Merged May 23, 2019
pull bot pushed a commit to Cache-Cloud/tensorflow that referenced this pull request May 23, 2019
@tensorflower-gardener
Merge pull request tensorflow#28686 from ThisIsIsaac:adjust_hsv_nhwc_fix
67b7636 
PiperOrigin-RevId: 249609869
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chsigg chsigg chsigg approved these changes

Assignees

@gbaned gbaned

Labels
cla: yes prtype:bugfix PR to fix a bug ready to pull PR ready for merge process size:M CL Change Size: Medium
Projects
PR Queue
  
Merged
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@ThisIsIsaac @chsigg @googlebot @kokoro-team @tensorflow-copybara @gbaned