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

Fix threshold optimizations in WIT #2682

Merged
merged 2 commits into from
Sep 20, 2019
Merged

Conversation

jameswex
Copy link
Contributor

  • Motivation for features / changes

Polymer2 update broke the updating of threshold sliders in WIT when certain optimization buttons are pressed.

  • Technical description of changes

Simplified threshold setting data-binding logic. No longer is there a map and a list to the same threshold information that is data-bound to paper-sliders. Now there is a simple list and the map is still used to quickly find items in the list by slice, but the map contains an index to the list and not the same object as is contained in the list. This fixes the issue where the updates to the slider bound values doesn't always cause the physical slider to update its setting.

  • Detailed steps to verify changes work correctly (as executed by you)

Ran through demos apps, sliced performance statistics by a feature, and verified that all threshold optimization buttons now correctly update the sliders, whereas before the sliders would stay stuck in one position even though other information would correctly update.

@jameswex
Copy link
Contributor Author

@tolga-b please review

Copy link
Contributor

@tolga-b tolga-b left a comment

Choose a reason for hiding this comment

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

Looks good to me James.

@jameswex jameswex merged commit 393ee8f into tensorflow:master Sep 20, 2019
@jameswex jameswex deleted the threshold_fix branch September 20, 2019 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants