Skip to content

Commit

Permalink
fix: no data plot in chart with data (#8935)
Browse files Browse the repository at this point in the history
* fix: `no data plot` in chart with data

* fix: fault torelant

* chore: Revert "fix: fault torelant"

This reverts commit ab00628.

* chore: add comment

(cherry picked from commit 77d1ede)
  • Loading branch information
keita-determined authored and determined-ci committed Mar 4, 2024
1 parent 5a74e37 commit f939a0f
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 14 deletions.
17 changes: 7 additions & 10 deletions webui/react/src/hooks/useMetricNames.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,32 +10,29 @@ import { metricKeyToMetric, metricSorter, metricToKey } from 'utils/metric';

import usePrevious from './usePrevious';

// DO NOT pass a raw object for experimentIds param
// That causes unwanted API call
const useMetricNames = (
experimentIds: number[],
errorHandler?: (e: unknown) => void,
quickPoll?: boolean,
): Loadable<Metric[]> => {
const [metrics, setMetrics] = useState<Loadable<Metric[]>>(NotLoaded);
const [actualExpIds, setActualExpIds] = useState<number[]>([]);
const previousExpIds = usePrevious(actualExpIds, []);
useEffect(
() => setActualExpIds((prev) => (_.isEqual(prev, experimentIds) ? prev : experimentIds)),
[experimentIds],
);
const previousExpIds = usePrevious(experimentIds, []);

useEffect(() => {
if (actualExpIds.length === 0) {
if (experimentIds.length === 0) {
setMetrics(Loaded([]));
return;
}
if (!_.isEqual(actualExpIds, previousExpIds)) setMetrics(NotLoaded);
if (!_.isEqual(experimentIds, previousExpIds)) setMetrics(NotLoaded);
const canceler = new AbortController();

// We do not want to plot any x-axis metric values as y-axis data
const xAxisMetrics = Object.values(XAxisDomain).map((v) => v.toLowerCase());

readStream<V1ExpMetricNamesResponse>(
detApi.StreamingInternal.expMetricNames(actualExpIds, quickPoll ? 5 : undefined, {
detApi.StreamingInternal.expMetricNames(experimentIds, quickPoll ? 5 : undefined, {
signal: canceler.signal,
}),
(event: V1ExpMetricNamesResponse) => {
Expand Down Expand Up @@ -82,7 +79,7 @@ const useMetricNames = (
errorHandler,
);
return () => canceler.abort();
}, [actualExpIds, previousExpIds, errorHandler, quickPoll]);
}, [experimentIds, previousExpIds, errorHandler, quickPoll]);

return metrics;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,10 @@ const ExperimentVisualization: React.FC<Props> = ({ basePath, experiment }: Prop
setPageError(PageError.MetricNames);
}, []);

const experimentIds = useMemo(() => [experiment.id], [experiment.id]);

// Stream available metrics.
const loadableMetrics = useMetricNames([experiment.id], handleMetricNamesError);
const loadableMetrics = useMetricNames(experimentIds, handleMetricNamesError);
const metrics = Loadable.getOrElse([], loadableMetrics);

const { hasData, hasLoaded, isExperimentTerminal, isSupported } = useMemo(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,10 @@ const MultiTrialDetailsHyperparameters: React.FC<Props> = ({
],
);

const experimentIds = useMemo(() => [experiment.id], [experiment.id]);

// Stream available metrics.
const loadableMetrics = useMetricNames([experiment.id], handleError);
const loadableMetrics = useMetricNames(experimentIds, handleError);
const metrics = Loadable.getOrElse([], loadableMetrics);

const isSupported = useMemo(() => {
Expand Down
8 changes: 6 additions & 2 deletions webui/react/src/pages/TrialDetails/TrialDetailsOverview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,17 @@ const TrialDetailsOverview: React.FC<Props> = ({ experiment, trial }: Props) =>

const trialNonTerminal = !terminalRunStates.has(experiment.state ?? RunState.Error);

const experimentIds = useMemo(() => [experiment.id], [experiment.id]);
const loadableMetricNames = useMetricNames(
[experiment.id],
experimentIds,
handleMetricNamesError,
trialNonTerminal,
);

const defaultMetricNames = useMemo(() => [], []);
const metricNames = Loadable.getOrElse(defaultMetricNames, loadableMetricNames);
const metricNames = useMemo(() => {
return Loadable.getOrElse(defaultMetricNames, loadableMetricNames);
}, [defaultMetricNames, loadableMetricNames]);

const { defaultMetrics, metrics } = useMemo(() => {
const validationMetric = experiment?.config?.searcher.metric;
Expand Down

0 comments on commit f939a0f

Please sign in to comment.