Skip to content

Commit

Permalink
fix: revert config work from #8765 and #8789 due to feature regressio…
Browse files Browse the repository at this point in the history
…ns (#8849)

(cherry picked from commit 47061fa)
  • Loading branch information
keita-determined authored and determined-ci committed Feb 16, 2024
1 parent 1888b90 commit 3129d33
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 87 deletions.
59 changes: 13 additions & 46 deletions webui/react/src/components/ExperimentActionDropdown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,22 @@ import Button from 'hew/Button';
import Dropdown, { DropdownEvent, MenuItem } from 'hew/Dropdown';
import Icon from 'hew/Icon';
import { useModal } from 'hew/Modal';
import Spinner from 'hew/Spinner';
import { useToast } from 'hew/Toast';
import useConfirm from 'hew/useConfirm';
import { copyToClipboard } from 'hew/utils/functions';
import { Loadable, NotLoaded } from 'hew/utils/loadable';
import React, { MouseEvent, useCallback, useMemo } from 'react';

import css from 'components/ActionDropdown/ActionDropdown.module.scss';
import ExperimentEditModalComponent from 'components/ExperimentEditModal';
import ExperimentMoveModalComponent from 'components/ExperimentMoveModal';
import HyperparameterSearchModalComponent from 'components/HyperparameterSearchModal';
import { useAsync } from 'hooks/useAsync';
import usePermissions from 'hooks/usePermissions';
import { handlePath } from 'routes/utils';
import {
activateExperiment,
archiveExperiment,
cancelExperiment,
deleteExperiment,
getExperiment,
killExperiment,
openOrCreateTensorBoard,
pauseExperiment,
Expand Down Expand Up @@ -93,23 +89,6 @@ const ExperimentActionDropdown: React.FC<Props> = ({
const confirm = useConfirm();
const { openToast } = useToast();

// this is required when experiment does not contain `config`.
// since we removed config. See #8765 on GitHub
const fetchedExperimentItem: Loadable<ExperimentItem> = useAsync(
async (canceler) => {
try {
const response = await getExperiment({ id: experiment.id }, { signal: canceler.signal });
return response;
} catch (e) {
handleError(e, { publicSubject: 'Unable to fetch experiment.' });
return NotLoaded;
}
},
[experiment.id],
);

const experimentItem: ExperimentItem = fetchedExperimentItem.getOrElse(experiment);

const handleEditComplete = useCallback(
(data: Partial<ExperimentItem>) => {
onComplete?.(ExperimentAction.Edit, id, data);
Expand Down Expand Up @@ -304,40 +283,28 @@ const ExperimentActionDropdown: React.FC<Props> = ({
/>
<HyperparameterSearchModal.Component
closeModal={HyperparameterSearchModal.close}
experiment={experimentItem}
experiment={experiment}
/>
</>
);

return children ? (
<>
{Loadable.isNotLoaded(fetchedExperimentItem) ? (
<Spinner center />
) : (
<>
<Dropdown
isContextMenu={isContextMenu}
menu={dropdownMenu}
open={makeOpen}
onClick={handleDropdown}>
{children}
</Dropdown>
{shared}
</>
)}
<Dropdown
isContextMenu={isContextMenu}
menu={dropdownMenu}
open={makeOpen}
onClick={handleDropdown}>
{children}
</Dropdown>
{shared}
</>
) : (
<div className={css.base} title="Open actions menu">
{Loadable.isNotLoaded(fetchedExperimentItem) ? (
<Spinner center />
) : (
<>
<Dropdown menu={dropdownMenu} placement="bottomRight" onClick={handleDropdown}>
<Button icon={<Icon name="overflow-vertical" size="small" title="Action menu" />} />
</Dropdown>
{shared}
</>
)}
<Dropdown menu={dropdownMenu} placement="bottomRight" onClick={handleDropdown}>
<Button icon={<Icon name="overflow-vertical" size="small" title="Action menu" />} />
</Dropdown>
{shared}
</div>
);
};
Expand Down
43 changes: 10 additions & 33 deletions webui/react/src/pages/F_ExpList/CompareParallelCoordinates.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,17 @@ import Hermes, { DimensionType } from 'hermes-parallel-coordinates';
import Alert from 'hew/Alert';
import Message from 'hew/Message';
import Spinner from 'hew/Spinner';
import { Loadable, NotLoaded } from 'hew/utils/loadable';
import React, { useCallback, useEffect, useMemo, useState } from 'react';

import ParallelCoordinates from 'components/ParallelCoordinates';
import Section from 'components/Section';
import { useAsync } from 'hooks/useAsync';
import { useGlasbey } from 'hooks/useGlasbey';
import { useSettings } from 'hooks/useSettings';
import { ExperimentVisualizationType } from 'pages/ExperimentDetails/ExperimentVisualization';
import ExperimentVisualizationFilters, {
VisualizationFilters,
} from 'pages/ExperimentDetails/ExperimentVisualization/ExperimentVisualizationFilters';
import { TrialMetricData } from 'pages/TrialDetails/useTrialMetrics';
import { getExperiment } from 'services/api';
import {
ExperimentWithTrial,
HpTrialData,
Expand All @@ -29,7 +26,6 @@ import {
} from 'types';
import { defaultNumericRange, getNumericRange, updateRange } from 'utils/chart';
import { flattenObject, isPrimitive } from 'utils/data';
import handleError from 'utils/error';
import { metricToKey, metricToStr } from 'utils/metric';
import { numericSorter } from 'utils/sort';

Expand Down Expand Up @@ -135,33 +131,14 @@ const CompareParallelCoordinates: React.FC<Props> = ({
const selectedMetric = settings.metric;
const selectedHParams = settings.hParams;

const experimentHyperparameters = useAsync(
async (canceler) => {
try {
const getExperimentRequests = selectedExperiments.map((exp) => {
return getExperiment({ id: exp.experiment.id }, { signal: canceler.signal });
});
const responses = await Promise.all(getExperimentRequests);
const hyperparameters = responses.map(
(exp) => exp.config?.hyperparameters as Record<string, Hyperparameter>,
);
const hpMap: Record<string, Hyperparameter> = {};
hyperparameters.forEach((hyperparameter) => {
const hps = Object.keys(hyperparameter);
hps.forEach((hp) => {
hpMap[hp] = hyperparameter[hp];
});
});
return hpMap;
} catch (e) {
handleError(e, {
publicSubject: 'Unable to fetch selected experiments.',
});
return NotLoaded;
}
},
[selectedExperiments],
);
const experimentHyperparameters = useMemo(() => {
const hpMap: Record<string, Hyperparameter> = {};
selectedExperiments.forEach((exp) => {
const hps = Object.keys(exp.experiment.hyperparameters);
hps.forEach((hp) => (hpMap[hp] = exp.experiment.hyperparameters[hp]));
});
return hpMap;
}, [selectedExperiments]);

const config: Hermes.RecursivePartial<Hermes.Config> = useMemo(
() => ({
Expand Down Expand Up @@ -190,7 +167,7 @@ const CompareParallelCoordinates: React.FC<Props> = ({

const dimensions = useMemo(() => {
const newDimensions: Hermes.Dimension[] = selectedHParams.map((key) => {
const hp = experimentHyperparameters.getOrElse({})[key] || {};
const hp = experimentHyperparameters[key] || {};

if (hp.type === HyperparameterType.Categorical || hp.vals) {
return {
Expand Down Expand Up @@ -287,7 +264,7 @@ const CompareParallelCoordinates: React.FC<Props> = ({
});
}, [selectedExperiments, selectedMetric, fullHParams, metricData, selectedScale, trials, data]);

if (!isLoaded || Loadable.isNotLoaded(experimentHyperparameters)) {
if (!isLoaded) {
return <Spinner center spinning />;
}

Expand Down
2 changes: 1 addition & 1 deletion webui/react/src/services/apiConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -645,7 +645,7 @@ export const getExperiment: DetApi<
> = {
name: 'getExperiment',
postProcess: (response: Api.V1GetExperimentResponse) => {
const exp = decoder.mapV1Experiment(response.experiment, undefined, response.config);
const exp = decoder.mapV1Experiment(response.experiment, undefined);
return exp;
},
request: (params: Service.GetExperimentParams) => {
Expand Down
21 changes: 14 additions & 7 deletions webui/react/src/services/decoder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -430,11 +430,10 @@ export const encodeExperimentState = (state: types.RunState): Sdk.Experimentv1St
export const mapV1GetExperimentDetailsResponse = ({
experiment: exp,
jobSummary,
config,
}: Sdk.V1GetExperimentResponse): types.ExperimentBase => {
const ioConfig = ioTypes.decode<ioTypes.ioTypeExperimentConfig>(
ioTypes.ioExperimentConfig,
config,
exp.config,
);
const continueFn = (value: unknown) => !(value as types.HyperparameterBase).type;
const hyperparameters = flattenObject<types.HyperparameterBase>(ioConfig.hyperparameters, {
Expand All @@ -444,7 +443,7 @@ export const mapV1GetExperimentDetailsResponse = ({
return {
...v1Exp,
config: ioToExperimentConfig(ioConfig),
configRaw: config,
configRaw: exp.config,
hyperparameters,
originalConfig: exp.originalConfig,
parentArchived: exp.parentArchived ?? false,
Expand All @@ -467,21 +466,29 @@ export const mapSearchExperiment = (
export const mapV1Experiment = (
data: Sdk.V1Experiment,
jobSummary?: types.JobSummary,
config?: Sdk.V1GetExperimentResponse['config'],
): types.ExperimentItem => {
const ioConfig = ioTypes.decode<ioTypes.ioTypeExperimentConfig>(
ioTypes.ioExperimentConfig,
data.config,
);
const continueFn = (value: unknown) => !(value as types.HyperparameterBase).type;
const hyperparameters = flattenObject<types.HyperparameterBase>(ioConfig.hyperparameters, {
continueFn,
}) as types.HyperparametersFlattened;

return {
archived: data.archived,
checkpoints: data.checkpointCount,
checkpointSize: parseInt(data?.checkpointSize || '0'),
config: config ?? {},
configRaw: config,
config: ioToExperimentConfig(ioConfig),
configRaw: data.config,
description: data.description,
duration: data.duration,
endTime: data.endTime as unknown as string,
externalExperimentId: data.externalExperimentId,
externalTrialId: data.externalTrialId,
forkedFrom: data.forkedFrom,
hyperparameters: config?.hyperparameters ?? {},
hyperparameters,
id: data.id,
jobId: data.jobId,
jobSummary: jobSummary,
Expand Down

0 comments on commit 3129d33

Please sign in to comment.