upgrade config for experiment detail as well

hamidzr · Aug 13, 2020 · ee237da · ee237da
1 parent 9e09845
commit ee237da
Show file tree

Hide file tree

Showing 4 changed files with 80 additions and 65 deletions.
diff --git a/webui/react/src/pages/ExperimentDetails.tsx b/webui/react/src/pages/ExperimentDetails.tsx
@@ -29,6 +29,7 @@ import { clone } from 'utils/data';
 import { alphanumericSorter, numericSorter, runStateSorter, stringTimeSorter } from 'utils/data';
 import { humanReadableFloat } from 'utils/string';
 import { getDuration } from 'utils/time';
+import { upgradeConfig } from 'utils/types';
 
 import css from './ExperimentDetails.module.scss';
 
@@ -55,23 +56,34 @@ const ExperimentDetailsComp: React.FC = () => {
   const [ forkModalVisible, setForkModalVisible ] = useState(false);
   const [ forkModalConfig, setForkModalConfig ] = useState('Loading');
 
+  const setFreshForkConfig = useCallback(() => {
+    if (!experiment?.configRaw) return;
+    // do not reset the config if the modal is open
+    if (forkModalVisible) return;
+    const prefix = 'Fork of ';
+    const rawConfig = clone(experiment.configRaw);
+    rawConfig.description = prefix + rawConfig.description;
+    upgradeConfig(rawConfig);
+    setForkModalConfig(yaml.safeDump(rawConfig));
+  }, [ experiment?.configRaw, forkModalVisible ]);
+
+  const handleForkModalCancel = useCallback(() => {
+    setForkModalVisible(false);
+    setFreshForkConfig();
+  }, [ setFreshForkConfig ]);
+
   useEffect(() => {
-    if (experiment && experiment.config) {
-      try {
-        const prefix = 'Fork of ';
-        const rawConfig = clone(experiment.configRaw);
-        rawConfig.description = prefix + rawConfig.description;
-        setForkModalConfig(yaml.safeDump(rawConfig));
-      } catch (e) {
-        handleError({
-          error: e,
-          message: 'failed to load experiment config',
-          type: ErrorType.ApiBadResponse,
-        });
-        setForkModalConfig('failed to load experiment config');
-      }
+    try {
+      setFreshForkConfig();
+    } catch (e) {
+      handleError({
+        error: e,
+        message: 'failed to load experiment config',
+        type: ErrorType.ApiBadResponse,
+      });
+      setForkModalConfig('failed to load experiment config');
     }
-  }, [ experiment ]);
+  }, [ setFreshForkConfig ]);
 
   const showForkModal = useCallback((): void => {
     setForkModalVisible(true);
@@ -236,6 +248,7 @@ const ExperimentDetailsComp: React.FC = () => {
         parentId={id}
         title={`Fork Experiment ${id}`}
         visible={forkModalVisible}
+        onCancel={handleForkModalCancel}
         onConfigChange={setForkModalConfig}
         onVisibleChange={setForkModalVisible}
       />

diff --git a/webui/react/src/pages/TrialDetails.tsx b/webui/react/src/pages/TrialDetails.tsx
@@ -21,58 +21,27 @@ import { getExperimentDetails, getTrialDetails, isNotFound } from 'services/api'
 import { TrialDetailsParams } from 'services/types';
 import { ExperimentDetails, RawJson, TrialDetails } from 'types';
 import { clone } from 'utils/data';
-import { trialHParamsToExperimentHParams } from 'utils/types';
+import { getLengthFromStepCount, trialHParamsToExperimentHParams, upgradeConfig } from 'utils/types';
 
 interface Params {
   trialId: string;
 }
 
-const DEFAULT_BATCHES_PER_STEP = 100;
-
-const getLengthFromStepCount = (config: RawJson, stepCount: number): [string, number] => {
-  // provide backward compat for step count
-  const batchesPerStep = config.batches_per_step || DEFAULT_BATCHES_PER_STEP;
-  return [ 'batches', stepCount * batchesPerStep ];
-};
-
 const getTrialLength = (config: RawJson): [string, number] => {
   const maxLength = config.searcher.max_length;
-  if (!maxLength) {
-    return getLengthFromStepCount(config, config.searcher.max_steps);
-  }
   return Object.entries(maxLength)[0] as [string, number];
 };
 
 const setTrialLength = (experimentConfig: RawJson, length: number): void => {
   const lengthType = getTrialLength(experimentConfig)[0];
   experimentConfig.searcher.max_length = { [lengthType]: length } ;
-  // in case we are dealing with an old config
-  delete experimentConfig.searcher.max_steps;
-};
-
-// Add opportunistic backward compatibility to old configs.
-const upgradeConfig = (config: RawJson): void => {
-  if (config.searcher.max_steps) {
-    const [ key, count ] = getLengthFromStepCount(config, config.searcher.max_steps);
-    config.searcher.max_length = { [key]: count };
-    delete config.searcher.max_steps;
-  }
-  if (typeof config.min_validation_period === 'number') {
-    const [ key, count ] = getLengthFromStepCount(config, config.min_validation_period);
-    config.searcher.min_validation_period = { [key]: count };
-  }
-  if (typeof config.min_checkpoint_period === 'number') {
-    const [ key, count ] = getLengthFromStepCount(config, config.min_checkpoint_period);
-    config.searcher.min_checkpoint_period = { [key]: count };
-  }
 };
 
 const trialContinueConfig =
 (experimentConfig: RawJson, trialHparams: Record<string, string>, trialId: number): RawJson => {
 
   return {
     ...experimentConfig,
-    description: experimentConfig.description,
     hyperparameters: trialHParamsToExperimentHParams(trialHparams),
     searcher: {
       max_length: experimentConfig.searcher.max_length,
@@ -116,6 +85,8 @@ const TrialDetailsComp: React.FC = () => {
 
   const setFreshContinueConfig = useCallback(() => {
     if (!experiment?.configRaw || !hparams) return;
+    // do not reset the config if the modal is open
+    if (contModalVisible || contFormVisible) return;
     const config = clone(experiment.configRaw);
 
     const newDescription = `Continuation of trial ${trialId}, experiment` +
@@ -129,14 +100,19 @@ const TrialDetailsComp: React.FC = () => {
     upgradeConfig(config);
     const newConfig = trialContinueConfig(config, hparams, trialId);
     setContModalConfig(yaml.safeDump(newConfig));
-  }, [ hparams, trialId, experiment?.configRaw, experimentId ]);
+  }, [ hparams, trialId, experiment?.configRaw, experimentId, contFormVisible, contModalVisible ]);
 
-  const handleContFormOnCancel = useCallback(() => {
+  const handleContFormCancel = useCallback(() => {
     setContFormVisible(false);
     setFreshContinueConfig();
     form.resetFields();
   }, [ setFreshContinueConfig, form ]);
 
+  const handleContModalCancel = useCallback(() => {
+    setContModalVisible(false);
+    setFreshContinueConfig();
+  }, [ setFreshContinueConfig ]);
+
   const updateStatesFromForm = useCallback(() => {
     if (!hparams || !trialId) return;
     const formValues = form.getFieldsValue();
@@ -268,7 +244,7 @@ If the problem persists please contact support.',
         parentId={experiment.id}
         title={`Continue Trial ${trialId}`}
         visible={contModalVisible}
-        onCancel={setFreshContinueConfig}
+        onCancel={handleContModalCancel}
         onConfigChange={onConfigChange}
         onVisibleChange={setContModalVisible}
       />
@@ -282,7 +258,7 @@ If the problem persists please contact support.',
         }}
         title={`Continue Trial ${trialId} of Experiment ${experimentId}`}
         visible={contFormVisible}
-        onCancel={handleContFormOnCancel}
+        onCancel={handleContFormCancel}
       >
         <Form
           form={form}

diff --git a/webui/react/src/utils/data.ts b/webui/react/src/utils/data.ts
@@ -1,4 +1,4 @@
-import { CommandMisc, CommandState, RunState, State } from 'types';
+import { CommandMisc, CommandState, RawJson, RunState, State } from 'types';
 
 export const isMap = <T>(data: T): boolean => data instanceof Map;
 export const isNumber = <T>(data: T): boolean => typeof data === 'number';

diff --git a/webui/react/src/utils/types.ts b/webui/react/src/utils/types.ts
@@ -1,6 +1,6 @@
 import {
   AnyTask, Checkpoint, Command, CommandState, CommandType, Experiment, ExperimentHyperParams,
-  ExperimentItem, RecentCommandTask, RecentExperimentTask, RecentTask, RunState, Step,
+  ExperimentItem, RawJson, RecentCommandTask, RecentExperimentTask, RecentTask, RunState, Step,
 } from 'types';
 
 import { getDuration } from './time';
@@ -160,18 +160,6 @@ export const oneOfProperties = <T>(obj: any, props: string[]): T => {
   throw new Error('no matching property');
 };
 
-export const trialHParamsToExperimentHParams = (hParams: Record<string, unknown>)
-: ExperimentHyperParams => {
-  const experimentHParams: ExperimentHyperParams = {};
-  Object.entries(hParams).forEach(([ param, value ]) => {
-    experimentHParams[param] = {
-      type: 'const',
-      val: value,
-    };
-  });
-  return experimentHParams;
-};
-
 // size in bytes
 export const checkpointSize = (checkpoint: Checkpoint): number => {
   const total = Object.values(checkpoint.resources).reduce((acc, size) => acc + size, 0);
@@ -198,3 +186,41 @@ export const trialDurations = (steps: Step[]): TrialDurations => {
     return acc;
   }, initialDurations);
 };
+
+/* Experiment Config */
+export const trialHParamsToExperimentHParams = (hParams: Record<string, unknown>)
+: ExperimentHyperParams => {
+  const experimentHParams: ExperimentHyperParams = {};
+  Object.entries(hParams).forEach(([ param, value ]) => {
+    experimentHParams[param] = {
+      type: 'const',
+      val: value,
+    };
+  });
+  return experimentHParams;
+};
+
+export const getLengthFromStepCount = (config: RawJson, stepCount: number): [string, number] => {
+  const DEFAULT_BATCHES_PER_STEP = 100;
+  // provide backward compat for step count
+  const batchesPerStep = config.batches_per_step || DEFAULT_BATCHES_PER_STEP;
+  return [ 'batches', stepCount * batchesPerStep ];
+};
+
+// Add opportunistic backward compatibility to old configs.
+export const upgradeConfig = (config: RawJson): void => {
+  if (config.searcher.max_steps) {
+    const [ key, count ] = getLengthFromStepCount(config, config.searcher.max_steps);
+    config.searcher.max_length = { [key]: count };
+    delete config.searcher.max_steps;
+    delete config.batches_per_step;
+  }
+  if (typeof config.min_validation_period === 'number') {
+    const [ key, count ] = getLengthFromStepCount(config, config.min_validation_period);
+    config.min_validation_period = { [key]: count };
+  }
+  if (typeof config.min_checkpoint_period === 'number') {
+    const [ key, count ] = getLengthFromStepCount(config, config.min_checkpoint_period);
+    config.min_checkpoint_period = { [key]: count };
+  }
+};