Skip to content

Commit

Permalink
Two fixes to debugger plugin (#1014)
Browse files Browse the repository at this point in the history
* Two fixes to debugger plugin

1. In some cases, a node with other nodes whose name scopes are the
   nodes name will not have a normalized (i.e., base-expanded) node
   name in the graph visualizer. E.g., consider a graph with two
   nodes, "Reshape" and "Reshape/shape". The "Reshape" node will
   be named as simply "Reshape", instead of "Reshape/(Reshape)" in
   the graph visualizer. As such, assuming base expansion always
   happens will lead to problems focusing on graph nodes through
   clicking nodes in the Node List or the Source Code View. This
   CL relaxes this assumption, which ensures the correct behavior.

2. When continuing to a given tensor, notify the user when a
   new Session Run has started. Also give user the option to stop
   continuation during the continue-to-tensor actions.
  • Loading branch information
caisq authored Mar 4, 2018
1 parent 5d0c749 commit e547f78
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ <h2>Continue...</h2>
// stopped, either because it reached the end, or because of forced
// stopping.
notifyContinuationStop() {
this._updateContinueButtonText(false);
this.updateContinueButtonText(false);
},

_openDialog() {
Expand All @@ -208,7 +208,7 @@ <h2>Continue...</h2>
// Update the text of the continue button.
// Args:
// active: Whether continuation is currently ongoing.
_updateContinueButtonText(active) {
updateContinueButtonText(active) {
this.set(
'_continueButtonText',
active ? this._continueButtonStopText : this._continueButtonContinueText);
Expand All @@ -217,7 +217,7 @@ <h2>Continue...</h2>
_sessionRunGoButtonCallback() {
if (this.continueNum > 0) {
this.sessionRunGo(this.continueNum);
this._updateContinueButtonText(true);
this.updateContinueButtonText(true);
this._closeDialog();
} else {
this.set('continueNum', 1);
Expand All @@ -241,7 +241,7 @@ <h2>Continue...</h2>
return;
}
this.tensorConditionGo(tensorConditionKey, refValue);
this._updateContinueButtonText(true);
this.updateContinueButtonText(true);
this._closeDialog();
},

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,12 @@
type: String,
value: null,
},
// Counts Session Runs in aggregation, regardless of fetch/feed/target.
_sessionRunTotalCounter: {
type: Number,
value: 0,
},
// Counts Session Runs by their fetch/feed/target combinations.
_sessionRunCounters: {
type: Object,
value: {},
Expand Down Expand Up @@ -651,6 +657,7 @@
} else {
this._sessionRunCounters[runInfo] = 1;
}
this._sessionRunTotalCounter++;
this.$.initialDialog.closeDialog();

// Do not refresh node list if we are in continue model and the
Expand All @@ -659,7 +666,7 @@
!this._continueToType || !_.isEqual(previousRunKey, runKey);
if (toProcessNewSessionRun) {
this._processGatedGrpcDebugOps(runKey, false);
this._showToast('A new Session.run() has begun.');
this._announceNewSessionRun();
}
} else if (responseType === 'tensor') {
const deviceName = responseData['device_name'];
Expand Down Expand Up @@ -732,7 +739,7 @@
// condition, it'll stop the continuation by setting _continueStop
// to true.
} else if (this._continueToType === 'op') {
this._processContinueToOp(responseData);
this._processContinueToOp(responseType === 'meta', responseData);
} else if (this._continueToType != null && this._continueToType !== '') {
console.error('Invalid _continueToType:', this._continueToType);
}
Expand All @@ -750,21 +757,26 @@
}
},

_processContinueToOp(responseData) {
_processContinueToOp(isSessionRunBeginning, responseData) {
if (isSessionRunBeginning) {
// A new Session.run() has started, which implies that the requested
// tensor was not hit in the previous Session.run, after the
// continue-to-tensor action was initiated. Notify the user as such.
this._announceNewSessionRun();
}
const deviceName = responseData['device_name'];
const maybeBaseExpandedNodeName = responseData[
'maybe_base_expanded_node_name'];
const nodeNameWithDevice = deviceName + '/' + maybeBaseExpandedNodeName;
if (nodeNameWithDevice !== this._continueToTarget) {
this._step();
} else {
const maybeBaseExpandedNodeName = responseData['maybe_base_expanded_node_name'];
const notBaseExpandedNodeName =
maybeBaseExpandedNodeName == null ? null :
tf_debugger_dashboard.removeNodeNameBaseExpansion(maybeBaseExpandedNodeName);
if (deviceName + '/' + maybeBaseExpandedNodeName === this._continueToTarget ||
deviceName + '/' + notBaseExpandedNodeName === this._continueToTarget) {
this._clearContinueTo();
if (this._sourceCodeShown) {
this.set(
'_sourceFocusNodeName',
tf_debugger_dashboard.removeNodeNameBaseExpansion(
maybeBaseExpandedNodeName));
this.set('_sourceFocusNodeName', notBaseExpandedNodeName);
}
} else {
this._step();
}
},

Expand Down Expand Up @@ -820,6 +832,10 @@
this.$.toast.setAttribute('text', text);
this.$.toast.open();
},
_announceNewSessionRun() {
this._showToast(
'Session.run() #' + this._sessionRunTotalCounter + ' is starting.');
},

_displayGraph(runKey, deviceName) {
const parameters = {
Expand Down Expand Up @@ -932,6 +948,15 @@
graph.panToNode(nodeName);
} else {
// Selecting the node triggers a pan to it.
const nodeMap = graph.get('renderHierarchy').hierarchy.getNodeMap();
if (!nodeMap[nodeName]) {
// In some cases, a node with other names using its name as
// name scope may not have base-expanded (i.e., normalized) name
// in the graph visualizer. In that case, we use the non-expanded
// node name.
nodeName =
tf_debugger_dashboard.removeNodeNameBaseExpansion(nodeName);
}
graph.set('selectedNode', nodeName);
}
this.set('_highlightNodeName', deviceName + '/' + nodeName);
Expand Down Expand Up @@ -1122,6 +1147,7 @@
tf_debugger_dashboard.removeNodeNameBaseExpansion(cleanNodeName));
}
this._setContinueTo('op', nodeNameWithDevice);
this.$.continueDialog.updateContinueButtonText(true);
this._step();
},

Expand Down

0 comments on commit e547f78

Please sign in to comment.