Skip to content

Commit 0ba10f7

Browse files
authored
hparams: Include hparams without filterable values in session group requests (#5971)
=== Motivation for features / changes === There are bugs in the hparams dashboard (Googlers, see b/239749612 and b/239749837) where the root cause appears to be the fact that some hparams are excluded from session_groups requests. Attempting to sort by the excluded column or any column to the right of it, column "i", would in fact sort by column "i+1". We have an "off-by-one" error. The code that excludes the column from the session_groups request is here: * https://cs.opensource.google/tensorflow/tensorboard/+/master:tensorboard/plugins/hparams/tf_hparams_query_pane/tf-hparams-query-pane.ts;l=822-829;drc=dc4754cf27d55c289b85a5ae77591f8d96fb682b * The column is being excluded because its metadata does not include a filter. Digging further, we see that this particular column is excluded because it is deemed to have too many possible discrete values: * https://cs.opensource.google/tensorflow/tensorboard/+/master:tensorboard/plugins/hparams/backend_context.py;l=270;drc=049f875d96c19afe439d0ee4d13f78c7b599169a === Technical description of changes === The fix proposed here is to delete the code that omits these columns from the request. Trying to guess the history here, it seems like the code was added when this was a clear error condition but later we added cases where this is expected. Examining the backend code, this seems like a safe change. * The backend code already handles the case where a column does not contain a filter: * https://cs.opensource.google/tensorflow/tensorboard/+/master:tensorboard/plugins/hparams/list_session_groups.py;l=439-474;drc=6996d71b9e0bad19a895a37a8bea8d7807775c39 * A column missing a filter indicates that no values should be filtered. This is the same behavior as just not including the column at all. * The backend code would already return a column in the response even if that column was not specified in the request. So including the column in the request when it previously was not included does not seem to have any impact on the response. === Detailed steps to verify changes work correctly (as executed by you) === 1. I generated an hparams data set with this colab notebook: * https://colab.research.google.com/drive/1FoMTeuFdoeMTnlKrb6vrvSOmLTjkX6ga#scrollTo=TB0wBWfcVqHz * It contains one column (kernel_init) with 12 discrete values. 2. Load a tensorboard with the logs and: * Verify kernel_init is now being included in the request and the data being returned in the response is still the complete set of hparams (that is, we aren't somehow accidentaly filtering results we weren't before). * Specify a filter for some other columns and verify that the response contains the expected rows. * Verify the kernel_init column can be sorted. * Verify that other columns can also be sorted and that there is no longer an off-by-one column error. * Specify a filter and download the data as CSV and ensure the correct data is included in the CSV (data download depends on the session_groups code). 3. Repeat the same verification with a TensorBoard.corp instance.
1 parent 6651465 commit 0ba10f7

File tree

1 file changed

+0
-7
lines changed

1 file changed

+0
-7
lines changed

Diff for: tensorboard/plugins/hparams/tf_hparams_query_pane/tf-hparams-query-pane.ts

-7
Original file line numberDiff line numberDiff line change
@@ -819,13 +819,6 @@ class TfHparamsQueryPane extends LegacyElementMixin(PolymerElement) {
819819
);
820820
} else if (hparam.filter.regexp) {
821821
colParam.filterRegexp = hparam.filter.regexp;
822-
} else {
823-
console.error(
824-
'hparam.filter with no domainDiscrete, interval or regexp' +
825-
' properties set: %s',
826-
hparam
827-
);
828-
return null;
829822
}
830823
colParams.push(colParam);
831824
});

0 commit comments

Comments
 (0)