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 saliency viewing in example viewer #1472

Merged
merged 22 commits into from
Oct 1, 2018
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
d84a0eb
Add flags to TBContext
jameswex Mar 19, 2018
ab0e9cf
Add flags to TBContext in application
jameswex Mar 19, 2018
e72cbcf
Add flags to tbcontext
jameswex Mar 19, 2018
3803ab7
gitignore changes
jameswex Apr 11, 2018
b65f56c
Merge branch 'master' of https://github.com/tensorflow/tensorboard
jameswex Apr 11, 2018
77a5109
Merge remote-tracking branch 'upstream/master'
jameswex Apr 13, 2018
92e5a72
Merge remote-tracking branch 'upstream/master'
jameswex Apr 18, 2018
0611086
Merge branch 'master' of https://github.com/tensorflow/tensorboard
jameswex Apr 20, 2018
2758a2a
Merge remote-tracking branch 'upstream/master'
jameswex May 9, 2018
8820f4c
Merge remote-tracking branch 'upstream/master'
jameswex May 9, 2018
d1299c6
Merge remote-tracking branch 'upstream/master'
jameswex Aug 6, 2018
c0c8258
jwexler updated rules_closure version
jameswex Aug 6, 2018
019e6f7
Merge remote-tracking branch 'upstream/master'
jameswex Sep 5, 2018
1833317
Merge remote-tracking branch 'upstream/master'
jameswex Sep 10, 2018
286df31
Merge branch 'master' of https://github.com/tensorflow/tensorboard
jameswex Sep 14, 2018
4cebc83
Merge branch 'master' of https://github.com/jameswex/tensorboard
jameswex Sep 17, 2018
d37b198
Merge remote-tracking branch 'upstream/master'
jameswex Sep 19, 2018
9335a8b
Merge remote-tracking branch 'upstream/master'
jameswex Sep 20, 2018
96fe3be
Merge remote-tracking branch 'upstream/master'
jameswex Sep 24, 2018
e2f8e08
Merge remote-tracking branch 'upstream/master'
jameswex Sep 28, 2018
7ccdb72
Fix saliency viewing in example viewer
jameswex Sep 28, 2018
a97d51b
review updates
jameswex Oct 1, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions tensorboard/components/vz_example_viewer/vz-example-viewer.html
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@
display: block;
padding: 8px;
width: fit-content;
border: 1px solid lightgrey;
}

.hide-saliency-controls {
Expand Down Expand Up @@ -474,7 +475,7 @@
value="[[getFirstFeatureValue(feat.name)]]">
</input>
<template is="dom-if" if="[[featureHasMultipleValues(feat.name)]]">
<paper-button data-feature="[[feat.name]]" on-click="expandFeature" class$="[[getInputPillClass(expandPillClass, displayMode)]]">
<paper-button data-feature="[[feat.name]]" on-click="expandFeature" class$="[[getInputPillClass(feat.name, displayMode)]]">
...
</paper-button>
</template>
Expand All @@ -498,7 +499,7 @@
value="[[getFirstCompareFeatureValue(feat.name)]]">
</input>
<template is="dom-if" if="[[compareFeatureHasMultipleValues(feat.name)]]">
<paper-button data-feature="[[feat.name]]" on-click="expandFeature" class$="[[getCompareInputClass(expandPillClass, displayMode)]]">
<paper-button data-feature="[[feat.name]]" on-click="expandFeature" class$="[[getCompareInputClass(feat.name, displayMode)]]">
...
</paper-button>
</template>
Expand Down Expand Up @@ -635,7 +636,7 @@
value="[[getFirstSeqFeatureValue(seqfeat.name, seqNumber)]]">
</input>
<template is="dom-if" if="[[seqFeatureHasMultipleValues(seqfeat.name, seqNumber)]]">
<paper-button data-feature="[[seqfeat.name]]" on-click="expandFeature" class$="[[getInputPillClass(expandPillClass, displayMode)]]">
<paper-button data-feature="[[seqfeat.name]]" on-click="expandFeature" class$="[[getInputPillClass(seqfeat.name, displayMode)]]">
...
</paper-button>
</template>
Expand All @@ -661,7 +662,7 @@
value="[[getFirstCompareSeqFeatureValue(seqfeat.name, seqNumber)]]">
</input>
<template is="dom-if" if="[[compareSeqFeatureHasMultipleValues(seqfeat.name, seqNumber)]]">
<paper-button data-feature="[[seqfeat.name]]" on-click="expandFeature" class$="[[getSeqCompareInputClass(expandPillClass, displayMode, seqNumber)]]">
<paper-button data-feature="[[seqfeat.name]]" on-click="expandFeature" class$="[[getSeqCompareInputClass(seqfeat.name, displayMode, seqNumber)]]">
...
</paper-button>
</template>
Expand All @@ -685,8 +686,8 @@
</template>
<paper-icon-button on-click="openAddFeatureDialog" icon="add" title="Add feature"
class$="[[getAddValueButtonClass(readonly)]]">
</paper-icon button>
<paper-card class$="[[getSaliencyControlsHolderClass(saliency)]]">
</paper-icon-button>
<div class$="[[getSaliencyControlsHolderClass(saliency)]]">
<div class="saliency-legend-label">Saliency legend</div>
<div class="flex-controls">
<svg id="saliencyLegend"></svg>
Expand All @@ -700,7 +701,7 @@
value="[[saliencyCutoff]]">
</paper-slider>
</div>
</paper-card>
</div>
</div>
<paper-icon-button id="deletevalue" icon="delete" class$="[[getDeleteValueButtonClass(readonly, showDeleteValueButton)]]"
data-feature="[[focusedFeatureName]]" data-index="[[focusedFeatureValueIndex]]"
Expand Down
93 changes: 67 additions & 26 deletions tensorboard/components/vz_example_viewer/vz-example-viewer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ const clipSaliencyRatio = .95;
// Colors for the saliency color scale.
const posSaliencyColor = '#0f0';
const negSaliencyColor = '#f00';
const neutralSaliencyColor = '#d3d3d3';
const neutralSaliencyColor = '#e8eaed';


const COLOR_INTERPOLATOR = d3.interpolateRgb;
Expand Down Expand Up @@ -129,7 +129,7 @@ Polymer({
ignoreChange: Boolean,
minSal: {type: Number, value: 0},
maxSal: {type: Number, value: 0},
showSaliency: {type: Boolean, value: false},
showSaliency: {type: Boolean, value: true},
imageInfo: {type: Object, value: {}},
windowWidth: {type: Number, value: DEFAULT_WINDOW_WIDTH},
windowCenter: {type: Number, value: DEFAULT_WINDOW_CENTER},
Expand All @@ -145,14 +145,13 @@ Polymer({
colors: {type: Object, computed: 'getColors(saliency)', observer: 'createLegend'},
displayMode: {type: String, value: 'grid'},
featureSearchValue: {type: String, value: '', notify: true},
filteredFeaturesList: {type: Object, computed: 'getFilteredFeaturesList(featuresList, featureSearchValue)'},
filteredSeqFeaturesList: {type: Object, computed: 'getFilteredFeaturesList(seqFeaturesList, featureSearchValue)'},
filteredFeaturesList: {type: Object, computed: 'getFilteredFeaturesList(featuresList, featureSearchValue, saliency)'},
filteredSeqFeaturesList: {type: Object, computed: 'getFilteredFeaturesList(seqFeaturesList, featureSearchValue, saliency)'},
focusedFeatureName: String,
focusedFeatureValueIndex: Number,
focusedSeqNumber: Number,
showDeleteValueButton: {type: Boolean, value: false},
expandedFeatures: {type: Object, value: {}},
expandPillClass: {type: String, value: 'expandPill'},
expandAllFeatures: {type: Boolean, value: false},
zeroIndex: {type: Number, value: 0},
compareJson: {type: Object, observer: 'createCompareExamplesFromJson'},
Expand All @@ -172,7 +171,7 @@ Polymer({
compareTitle: String,
},
observers: [
'haveSaliency(featuresList, saliency, colors, showSaliency, saliencyCutoff)',
'haveSaliency(filteredFeaturesList, saliency, colors, showSaliency, saliencyCutoff)',
'seqSaliency(seqNumber, seqFeaturesList, saliency, colors, showSaliency, saliencyCutoff)',
],

Expand Down Expand Up @@ -276,8 +275,20 @@ Polymer({
},

getFilteredFeaturesList: function(featureList: NameAndFeature[],
searchValue: string) {
searchValue: string, saliency: SaliencyMap) {
let filtered = featureList;
const checkSal = saliency && Object.keys(saliency).length > 0;
// Create a dict of feature names to the total absolute saliency of all
// its feature values, to sort features with the most salienct features at
// the top.
const saliencyTotals = checkSal ?
Object.assign({}, ...Object.keys(saliency).map(
Copy link
Contributor

Choose a reason for hiding this comment

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

This is doing Object.assign on objects that you map out of keys of saliency. I think you can do this either by cloning saliency object and transforming its values or

Object.entries(saliency)
	.map(([key, value]) => [key, typeof value == 'number' ? [value] : value])
	.map(([key, value]) => [key, value.map(Math.abs).reduce((total, cur) => total + cur, 0)])
        .reduce((obj, [key, value]) => {
           obj[key] = value;
           return obj;
        }, {})

though I am not sure if this has any better readability.

name => ({[name]: typeof saliency[name] == 'number' ?
Math.abs(saliency[name] as number) :
(saliency[name] as Array<number>).reduce((total, cur) =>
Math.abs(total) + Math.abs(cur) , 0)}))) :
{};

if (searchValue != '') {
const re = new RegExp(searchValue, 'i');
filtered = featureList.filter(feature => re.test(feature.name));
Expand All @@ -288,6 +299,18 @@ Polymer({
} else if (this.isImage(b.name) && !this.isImage(a.name)) {
return 1;
} else {
if (checkSal) {
if (a.name in saliency && !(b.name in saliency)) {
return -1;
} else if (b.name in saliency && !(a.name in saliency)) {
return 1;
} else {
const diff = saliencyTotals[b.name] - saliencyTotals[a.name];
if (diff != 0) {
return diff;
}
}
}
return a.name.localeCompare(b.name);
}
});
Expand Down Expand Up @@ -326,29 +349,33 @@ Polymer({
]);
},

selectAll: function(query: string) {
return d3.selectAll(
Polymer.dom(this.root).querySelectorAll(query) as any);
Copy link
Contributor

Choose a reason for hiding this comment

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

Just wondering, how does this differ from this.querySelectorAll?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

probably no different, was just following what i saw in the doc at https://www.polymer-project.org/1.0/docs/devguide/local-dom

},

haveSaliency: function() {
if (!this.featuresList || !this.saliency ||
if (!this.filteredFeaturesList || !this.saliency ||
Object.keys(this.saliency).length === 0 || !this.colors) {
return;
}

// TODO(jwexler): Find a way to do this without requestAnimationFrame.
Copy link
Contributor

Choose a reason for hiding this comment

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

Just saying, this seems to happen because of _debounceTemplate in dom-repeat: https://github.com/Polymer/polymer/blob/1.x/src/lib/template/dom-repeat.html#L492

// If the paper-inputs for the features have yet to be rendered, wait to
// perform this processing. There should be paper-inputs for all non-image
// If the inputs for the features have yet to be rendered, wait to
// perform this processing. There should be inputs for all non-image
// features.
if (d3.selectAll('.value input').size() <
(this.featuresList.length - Object.keys(this.imageInfo).length)) {
if (this.selectAll('input.value-pill').size() <
(this.filteredFeaturesList.length - Object.keys(this.imageInfo).length)) {
requestAnimationFrame(() => this.haveSaliency());
return;
}

// Reset all text to black
d3.selectAll<HTMLInputElement, {}>('.value-pill')
.style('background', 'lightgrey');

this.selectAll('.value-pill')
.style('background', '#e8eaed');
Copy link
Contributor

@stephanwlee stephanwlee Sep 28, 2018

Choose a reason for hiding this comment

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

I am not sure how this code relates to the comment in L373. Are they related? Also, would it be possible to hoist this constant color value as a module level constant (i.e., neutralSaliencyColor)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed comment and changed to use neutralSaliencyColor as it should

// Color the text of each input element of each feature according to the
// provided saliency information.
for (const feat of this.featuresList) {
for (const feat of this.filteredFeaturesList) {
const val = this.saliency[feat.name] as SaliencyValue;
// If there is no saliency information for the feature, do not color it.
if (!val) {
Expand All @@ -357,13 +384,26 @@ Polymer({
const colorFn = Array.isArray(val) ?
(d: {}, i: number) => this.getColorForSaliency(val[i]) :
() => this.getColorForSaliency(val);

d3.selectAll<HTMLInputElement, {}>(
`.${this.sanitizeFeature(feat.name)}.value-pill`)
.style('background', this.showSaliency ? colorFn : () => 'lightgrey');
this.selectAll(
`input.${this.sanitizeFeature(feat.name)}.value-pill`)
.style('background', this.showSaliency ? colorFn : () => '#e8eaed');

// Color the "more feature values" button with the most extreme saliency
// of any of the feature values hidden behind the button.
if (Array.isArray(val)) {
const valArray = val as Array<number>;
const moreButton = this.selectAll(
`paper-button.${this.sanitizeFeature(feat.name)}.value-pill`);
let mostExtremeSal = 0;
for (let i = 1; i < valArray.length; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

no action required: an alternative to write this is, Math.max(...valArray.map(Math.abs))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did this way so as to keep the most extreme true value from valArray instead of the abs value of it.

if (Math.abs(valArray[i]) > Math.abs(mostExtremeSal)) {
mostExtremeSal = valArray[i];
}
}
moreButton.style('background', this.showSaliency ?
() => this.getColorForSaliency(mostExtremeSal) : () => '#e8eaed');
}
}
// TODO(jwexler): Determine how to set non-fixed widths to input boxes
// inside of grid iron-list.
},

/**
Expand All @@ -382,7 +422,7 @@ Polymer({
// TODO(jwexler): Find a way to do this without requestAnimationFrame.
// If the paper-inputs for the features have yet to be rendered, wait to
// perform this processing.
if (d3.selectAll('.value input').size() < this.seqFeaturesList.length) {
if (this.selectAll('.value input').size() < this.seqFeaturesList.length) {
requestAnimationFrame(() => this.seqSaliency());
return;
}
Expand All @@ -401,7 +441,7 @@ Polymer({
(d: {}, i: number) => this.getColorForSaliency(val[i]) :
() => this.getColorForSaliency(val);

d3.selectAll<HTMLInputElement, {}>(
this.selectAll(
`.${this.sanitizeFeature(feat.name)} input`)
.style('color', this.showSaliency ? colorFn : () => 'black');
}
Expand Down Expand Up @@ -954,6 +994,7 @@ Polymer({
this.ignoreChange = false;
setTimeout(() => {
this.example = temp;
this.haveSaliency();
}, 0);
},

Expand Down Expand Up @@ -1057,7 +1098,7 @@ Polymer({
const legendSvg = d3.select(this.$.saliencyLegend).append('g');
const gradient = legendSvg.append('defs')
.append('linearGradient')
.attr('id', 'gradient')
.attr('id', 'vzexampleviewergradient')
.attr("x1", "0%")
.attr("y1", "0%")
.attr("x2", "100%")
Expand Down Expand Up @@ -1106,7 +1147,7 @@ Polymer({
.attr('y1', 0)
.attr('width', LEGEND_WIDTH_PX)
.attr('height', LEGEND_HEIGHT_PX)
.style('fill', 'url(#gradient)');
.style('fill', 'url(#vzexampleviewergradient)');

const legendScale =
d3.scaleLinear().domain([this.minSal, this.maxSal]).range([
Expand Down