Skip to content

Commit

Permalink
Hparam: Add selectors to retrieve data written by the new hparams eff…
Browse files Browse the repository at this point in the history
…ects (#6544)

## Motivation for features / changes
This is a continuation of #6540
Now that data is being retrieved from the hparams plugin and written to
the store I am adding a pair of selectors to retrieve it and inject it
into the the data retrieved from `getRuns` and `getRunsForExperimentId`.

The logic used to translate `SessionGroups` to `Record<RunId/Group,
Value>` is very similar to that being used by the old code in
`runs_data_source`. I could imagine this being refactored to retrieve
the run/group relationships from another part of the store in the future
in order to better integrate with our plans to stop relying on the
metrics portion of the hparams payload.

## Note For Reviewer(s)
A large majority of the lines changed in this PR are indent changes
resulting from changing the method used to mock the state during tests.

I have opted not to add additional test to the runs or common selectors
(though I did have to fix 16) as the intention is for this to be a drop
in replacement with no new features added. The number of test failures
which I had to fix left me feeling confident of the existing test
coverage.
  • Loading branch information
rileyajones authored Aug 21, 2023
1 parent fd3b82e commit b408561
Show file tree
Hide file tree
Showing 19 changed files with 783 additions and 258 deletions.
3 changes: 3 additions & 0 deletions tensorboard/webapp/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -142,13 +142,16 @@ tf_ng_module(
"app_state.ts",
],
deps = [
"//tensorboard/plugins/debugger_v2/tf_debugger_v2_plugin/store:types",
"//tensorboard/webapp/alert/store:types",
"//tensorboard/webapp/app_routing/store:types",
"//tensorboard/webapp/core/store",
"//tensorboard/webapp/experiments/store:types",
"//tensorboard/webapp/feature_flag/store:types",
"//tensorboard/webapp/hparams:types",
"//tensorboard/webapp/metrics/store:types",
"//tensorboard/webapp/notification_center/_redux:types",
"//tensorboard/webapp/persistent_settings/_redux:types",
"//tensorboard/webapp/runs/store:types",
"//tensorboard/webapp/settings",
],
Expand Down
1 change: 0 additions & 1 deletion tensorboard/webapp/alert/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ tf_ts_library(
"types.ts",
],
deps = [
"//tensorboard/webapp:app_state",
"@npm//@ngrx/store",
"@npm//rxjs",
],
Expand Down
5 changes: 2 additions & 3 deletions tensorboard/webapp/alert/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
==============================================================================*/
import {Action, Store} from '@ngrx/store';
import {State} from '../app_state';
import {Action} from '@ngrx/store';

/**
* An alert structure used when creating newly reported alerts.
Expand All @@ -35,7 +34,7 @@ export interface AlertReport {
* when the followup action is requested, a newly created Promise will be
* awaited, and the resulting action is dispatched.
*/
getFollowupAction: (store: Store<State>) => Promise<Action>;
getFollowupAction: () => Promise<Action>;
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,7 @@ export class AlertDisplaySnackbarContainer {
async onActionButtonClicked() {
this.snackBarRef.dismiss();

const followupAction = await this.alert.followupAction!.getFollowupAction(
this.store
);
const followupAction = await this.alert.followupAction!.getFollowupAction();
this.store.dispatch(followupAction);
}

Expand Down
8 changes: 7 additions & 1 deletion tensorboard/webapp/app_state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,16 @@ See the License for the specific language governing permissions and
limitations under the License.
==============================================================================*/

import {State as DebuggerState} from '../plugins/debugger_v2/tf_debugger_v2_plugin/store/debugger_types';
import {State as AlertState} from './alert/store/alert_types';
import {State as AppRoutingState} from './app_routing/store/app_routing_types';
import {State as CoreState} from './core/store/core_types';
import {State as ExperimentsState} from './experiments/store/experiments_types';
import {State as FeatureFlagState} from './feature_flag/store/feature_flag_types';
import {State as HparamsState} from './hparams/types';
import {State as MetricsState} from './metrics/store/metrics_types';
import {State as NotificationState} from './notification_center/_redux/notification_center_types';
import {State as PersistentSettingsState} from './persistent_settings/_redux/persistent_settings_types';
import {State as RunsState} from './runs/store/runs_types';
import {State as SettingsState} from './settings';

Expand All @@ -31,4 +34,7 @@ export type State = AppRoutingState &
MetricsState &
RunsState &
SettingsState &
NotificationState;
NotificationState &
DebuggerState &
AlertState &
PersistentSettingsState;
1 change: 1 addition & 0 deletions tensorboard/webapp/hparams/_redux/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ tf_ts_library(
"//tensorboard/webapp/runs/data_source:testing",
"//tensorboard/webapp/runs/store:testing",
"//tensorboard/webapp/testing:utils",
"//tensorboard/webapp/util:types",
"//tensorboard/webapp/webapp_data_source:http_client_testing",
"@npm//@ngrx/effects",
"@npm//@ngrx/store",
Expand Down
56 changes: 55 additions & 1 deletion tensorboard/webapp/hparams/_redux/hparams_selectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,13 @@ See the License for the specific language governing permissions and
limitations under the License.
==============================================================================*/
import {createFeatureSelector, createSelector} from '@ngrx/store';
import {DiscreteFilter, HparamAndMetricSpec, IntervalFilter} from '../types';
import {
DiscreteFilter,
HparamAndMetricSpec,
HparamValue,
IntervalFilter,
RunToHparamsAndMetrics,
} from '../types';
import {combineHparamAndMetricSpecs} from './hparams_selectors_utils';
import {HparamsState, HPARAMS_FEATURE_KEY} from './types';
import {
Expand Down Expand Up @@ -178,3 +184,51 @@ export const getExperimentsHparamsAndMetricsSpecs = createSelector(
);
}
);

export const getDashboardHparamsAndMetricsSpecs = createSelector(
getHparamsState,
(state: HparamsState) => {
return state.dashboardSpecs;
}
);

export const getDashboardRunsToHparamsAndMetrics = createSelector(
getHparamsState,
(state): RunToHparamsAndMetrics => {
const runToHparamsAndMetrics: RunToHparamsAndMetrics = {};

for (const sessionGroup of state.dashboardSessionGroups) {
const hparams: HparamValue[] = Object.entries(sessionGroup.hparams).map(
(keyValue) => {
const [hparam, value] = keyValue;
return {name: hparam, value};
}
);

for (const session of sessionGroup.sessions) {
runToHparamsAndMetrics[session.name] = {
metrics: [],
hparams,
};

for (const metricValue of session.metricValues) {
const runId = metricValue.name.group
? `${session.name}/${metricValue.name.group}`
: session.name;

const hparamsAndMetrics = runToHparamsAndMetrics[runId] || {
metrics: [],
hparams,
};
hparamsAndMetrics.metrics.push({
tag: metricValue.name.tag,
trainingStep: metricValue.trainingStep,
value: metricValue.value,
});
runToHparamsAndMetrics[runId] = hparamsAndMetrics;
}
}
}
return runToHparamsAndMetrics;
}
);
Loading

0 comments on commit b408561

Please sign in to comment.