Skip to content

Commit

Permalink
Refactor accelerator profile state hooks to no longer manage its own …
Browse files Browse the repository at this point in the history
…selected
  • Loading branch information
Gkrumbach07 committed Aug 16, 2024
1 parent a011b1b commit c9092cc
Show file tree
Hide file tree
Showing 12 changed files with 197 additions and 177 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ type ServingRuntimeDetailsProps = {

const ServingRuntimeDetails: React.FC<ServingRuntimeDetailsProps> = ({ obj, isvc }) => {
const { dashboardConfig } = React.useContext(AppContext);
const [acceleratorProfile] = useServingAcceleratorProfile(obj, isvc);
const acceleratorProfile = useServingAcceleratorProfile(obj, isvc);
const selectedAcceleratorProfile = acceleratorProfile.acceleratorProfile;
const enabledAcceleratorProfiles = acceleratorProfile.acceleratorProfiles.filter(
(ac) => ac.spec.enabled,
Expand Down Expand Up @@ -64,12 +64,12 @@ const ServingRuntimeDetails: React.FC<ServingRuntimeDetailsProps> = ({ obj, isvc
}`
: enabledAcceleratorProfiles.length === 0
? 'No accelerator enabled'
: acceleratorProfile.useExisting
: acceleratorProfile.unknownProfileDetected
? 'Unknown'
: 'No accelerator selected'}
</DescriptionListDescription>
</DescriptionListGroup>
{!acceleratorProfile.useExisting && acceleratorProfile.acceleratorProfile && (
{!acceleratorProfile.unknownProfileDetected && acceleratorProfile.acceleratorProfile && (
<DescriptionListGroup>
<DescriptionListTerm>Number of accelerators</DescriptionListTerm>
<DescriptionListDescription>{acceleratorProfile.count}</DescriptionListDescription>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,7 @@ const ManageServingRuntimeModal: React.FC<ManageServingRuntimeModalProps> = ({
editInfo,
}) => {
const [createData, setCreateData, resetData, sizes] = useCreateServingRuntimeObject(editInfo);
const [acceleratorProfileState, setAcceleratorProfileState, resetAcceleratorProfileData] =
useServingAcceleratorProfile(editInfo?.servingRuntime);
const acceleratorProfileState = useServingAcceleratorProfile(editInfo?.servingRuntime);
const [actionInProgress, setActionInProgress] = React.useState(false);
const [error, setError] = React.useState<Error | undefined>();

Expand Down Expand Up @@ -92,7 +91,6 @@ const ManageServingRuntimeModal: React.FC<ManageServingRuntimeModalProps> = ({
setError(undefined);
setActionInProgress(false);
resetData();
resetAcceleratorProfileData();
};

const setErrorModal = (e: Error) => {
Expand Down Expand Up @@ -180,7 +178,6 @@ const ManageServingRuntimeModal: React.FC<ManageServingRuntimeModalProps> = ({
sizes={sizes}
servingRuntimeSelected={servingRuntimeSelected}
acceleratorProfileState={acceleratorProfileState}
setAcceleratorProfileState={setAcceleratorProfileState}
infoContent="Select a server size that will accommodate your largest model. See the product documentation for more information."
/>
</StackItem>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ type ServingRuntimeSizeSectionProps = {
sizes: ModelServingSize[];
servingRuntimeSelected?: ServingRuntimeKind;
acceleratorProfileState: AcceleratorProfileState;
setAcceleratorProfileState: UpdateObjectAtPropAndValue<AcceleratorProfileState>;
infoContent?: string;
};

Expand All @@ -33,7 +32,6 @@ const ServingRuntimeSizeSection: React.FC<ServingRuntimeSizeSectionProps> = ({
sizes,
servingRuntimeSelected,
acceleratorProfileState,
setAcceleratorProfileState,
infoContent,
}) => {
const [supportedAcceleratorProfiles, setSupportedAcceleratorProfiles] = React.useState<
Expand Down Expand Up @@ -111,7 +109,6 @@ const ServingRuntimeSizeSection: React.FC<ServingRuntimeSizeSectionProps> = ({
<FormGroup>
<AcceleratorProfileSelectField
acceleratorProfileState={acceleratorProfileState}
setAcceleratorProfileState={setAcceleratorProfileState}
supportedAcceleratorProfiles={supportedAcceleratorProfiles}
resourceDisplayName="serving runtime"
infoContent="Ensure that appropriate tolerations are in place before adding an accelerator to your model server."
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,10 @@ const ManageKServeModal: React.FC<ManageKServeModalProps> = ({
const isInferenceServiceNameWithinLimit =
translateDisplayNameForK8s(createDataInferenceService.name).length <= 253;

const [acceleratorProfileState, setAcceleratorProfileState, resetAcceleratorProfileData] =
useServingAcceleratorProfile(
editInfo?.servingRuntimeEditInfo?.servingRuntime,
editInfo?.inferenceServiceEditInfo,
);
const acceleratorProfileState = useServingAcceleratorProfile(
editInfo?.servingRuntimeEditInfo?.servingRuntime,
editInfo?.inferenceServiceEditInfo,
);
const customServingRuntimesEnabled = useCustomServingRuntimesEnabled();
const [allowCreate] = useAccessReview({
...accessReviewResource,
Expand Down Expand Up @@ -187,7 +186,6 @@ const ManageKServeModal: React.FC<ManageKServeModalProps> = ({
setActionInProgress(false);
resetDataServingRuntime();
resetDataInferenceService();
resetAcceleratorProfileData();
setAlertVisible(true);
};

Expand Down Expand Up @@ -351,7 +349,6 @@ const ManageKServeModal: React.FC<ManageKServeModalProps> = ({
sizes={sizes}
servingRuntimeSelected={servingRuntimeSelected}
acceleratorProfileState={acceleratorProfileState}
setAcceleratorProfileState={setAcceleratorProfileState}
infoContent="Select a server size that will accommodate your largest model. See the product documentation for more information."
/>
</StackItem>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,11 @@ import { InferenceServiceKind, ServingRuntimeKind } from '~/k8sTypes';
import useAcceleratorProfileState, {
AcceleratorProfileState,
} from '~/utilities/useAcceleratorProfileState';
import { GenericObjectState } from '~/utilities/useGenericObjectState';

const useServingAcceleratorProfile = (
servingRuntime?: ServingRuntimeKind | null,
inferenceService?: InferenceServiceKind | null,
): GenericObjectState<AcceleratorProfileState> => {
): AcceleratorProfileState => {
const acceleratorProfileName =
servingRuntime?.metadata.annotations?.['opendatahub.io/accelerator-name'];
const resources =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,41 +17,53 @@ import { OutlinedQuestionCircleIcon } from '@patternfly/react-icons';
import { isHTMLInputElement } from '~/utilities/utils';
import { AcceleratorProfileKind } from '~/k8sTypes';
import SimpleSelect, { SimpleSelectOption } from '~/components/SimpleSelect';
import { UpdateObjectAtPropAndValue } from '~/pages/projects/types';
import { AcceleratorProfileState } from '~/utilities/useAcceleratorProfileState';
import useGenericObjectState from '~/utilities/useGenericObjectState';
import useDetectedAccelerators from './useDetectedAccelerators';

type AcceleratorProfileSelectFieldProps = {
acceleratorProfileState: AcceleratorProfileState;
setAcceleratorProfileState: UpdateObjectAtPropAndValue<AcceleratorProfileState>;
supportedAcceleratorProfiles?: string[];
resourceDisplayName?: string;
infoContent?: string;
};

const AcceleratorProfileSelectField: React.FC<AcceleratorProfileSelectFieldProps> = ({
acceleratorProfileState,
setAcceleratorProfileState,
supportedAcceleratorProfiles,
resourceDisplayName = 'image',
infoContent,
}) => {
const [detectedAccelerators] = useDetectedAccelerators();
const [
selectedAcceleratorProfile,
setSelectedAcceleratorProfile,
resetSelectedAcceleratorProfile,
] = useGenericObjectState<{
profile?: AcceleratorProfileKind;
count: number;
useExistingSettings: boolean;
}>({
profile: undefined,
count: 0,
useExistingSettings: false,
});

const {
acceleratorProfile,
count: acceleratorCount,
acceleratorProfiles,
useExisting,
additionalOptions,
} = acceleratorProfileState;
React.useEffect(() => {
setSelectedAcceleratorProfile('profile', acceleratorProfileState.acceleratorProfile);
setSelectedAcceleratorProfile('count', acceleratorProfileState.count);
setSelectedAcceleratorProfile(
'useExistingSettings',
acceleratorProfileState.unknownProfileDetected,
);
}, [acceleratorProfileState, setSelectedAcceleratorProfile]);

const generateAcceleratorCountWarning = (newSize: number) => {
if (!acceleratorProfile) {
if (!selectedAcceleratorProfile.profile) {
return '';
}

const { identifier } = acceleratorProfile.spec;
const { identifier } = selectedAcceleratorProfile.profile.spec;

const detectedAcceleratorCount = Object.entries(detectedAccelerators.available).find(
([id]) => identifier === id,
Expand All @@ -69,12 +81,14 @@ const AcceleratorProfileSelectField: React.FC<AcceleratorProfileSelectFieldProps
return '';
};

const acceleratorCountWarning = generateAcceleratorCountWarning(acceleratorCount);
const acceleratorCountWarning = generateAcceleratorCountWarning(selectedAcceleratorProfile.count);

const isAcceleratorProfileSupported = (cr: AcceleratorProfileKind) =>
supportedAcceleratorProfiles?.includes(cr.spec.identifier);

const enabledAcceleratorProfiles = acceleratorProfiles.filter((ac) => ac.spec.enabled);
const enabledAcceleratorProfiles = acceleratorProfileState.acceleratorProfiles.filter(
(ac) => ac.spec.enabled,
);

const formatOption = (cr: AcceleratorProfileKind): SimpleSelectOption => {
const displayName = `${cr.spec.displayName}${!cr.spec.enabled ? ' (disabled)' : ''}`;
Expand Down Expand Up @@ -112,13 +126,13 @@ const AcceleratorProfileSelectField: React.FC<AcceleratorProfileSelectFieldProps
.map((ac) => formatOption(ac));

let acceleratorAlertMessage: { title: string; variant: AlertVariant } | null = null;
if (acceleratorProfile && supportedAcceleratorProfiles !== undefined) {
if (selectedAcceleratorProfile.profile && supportedAcceleratorProfiles !== undefined) {
if (supportedAcceleratorProfiles.length === 0) {
acceleratorAlertMessage = {
title: `The ${resourceDisplayName} you have selected doesn't support the selected accelerator. It is recommended to use a compatible ${resourceDisplayName} for optimal performance.`,
variant: AlertVariant.info,
};
} else if (!isAcceleratorProfileSupported(acceleratorProfile)) {
} else if (!isAcceleratorProfileSupported(selectedAcceleratorProfile.profile)) {
acceleratorAlertMessage = {
title: `The ${resourceDisplayName} you have selected is not compatible with the selected accelerator`,
variant: AlertVariant.warning,
Expand All @@ -133,18 +147,21 @@ const AcceleratorProfileSelectField: React.FC<AcceleratorProfileSelectFieldProps
isPlaceholder: true,
});

if (additionalOptions?.useExisting) {
if (acceleratorProfileState.unknownProfileDetected) {
options.push({
key: 'use-existing',
label: 'Existing settings',
description: 'Use the existing accelerator settings from the notebook server',
});
} else if (additionalOptions?.useDisabled) {
options.push(formatOption(additionalOptions.useDisabled));
} else if (
selectedAcceleratorProfile.profile &&
!selectedAcceleratorProfile.profile.spec.enabled
) {
options.push(formatOption(selectedAcceleratorProfile.profile));
}

const onStep = (step: number) => {
setAcceleratorProfileState('count', Math.max(acceleratorCount + step, 1));
setSelectedAcceleratorProfile('count', Math.max(selectedAcceleratorProfile.count + step, 1));
};

// if there is more than a none option, show the dropdown
Expand All @@ -171,25 +188,29 @@ const AcceleratorProfileSelectField: React.FC<AcceleratorProfileSelectFieldProps
<SimpleSelect
isFullWidth
options={options}
value={useExisting ? 'use-existing' : acceleratorProfile?.metadata.name ?? ''}
value={
selectedAcceleratorProfile.useExistingSettings
? 'use-existing'
: selectedAcceleratorProfile.profile?.metadata.name ?? ''
}
onChange={(key, isPlaceholder) => {
if (isPlaceholder) {
// none
setAcceleratorProfileState('useExisting', false);
setAcceleratorProfileState('acceleratorProfile', undefined);
setAcceleratorProfileState('count', 0);
resetSelectedAcceleratorProfile();
} else if (key === 'use-existing') {
// use existing settings
setAcceleratorProfileState('useExisting', true);
setAcceleratorProfileState('acceleratorProfile', undefined);
setAcceleratorProfileState('count', 0);
setSelectedAcceleratorProfile('useExistingSettings', true);
setSelectedAcceleratorProfile('profile', undefined);
setSelectedAcceleratorProfile('count', 0);
} else {
// normal flow
setAcceleratorProfileState('count', 1);
setAcceleratorProfileState('useExisting', false);
setAcceleratorProfileState(
'acceleratorProfile',
acceleratorProfiles.find((ac) => ac.metadata.name === key),
setSelectedAcceleratorProfile('count', 1);
setSelectedAcceleratorProfile('useExistingSettings', false);
setSelectedAcceleratorProfile(
'profile',
acceleratorProfileState.acceleratorProfiles.find(
(ac) => ac.metadata.name === key,
),
);
}
}}
Expand All @@ -206,23 +227,23 @@ const AcceleratorProfileSelectField: React.FC<AcceleratorProfileSelectFieldProps
/>
</StackItem>
)}
{acceleratorProfile && (
{selectedAcceleratorProfile.profile && (
<StackItem>
<FormGroup label="Number of accelerators" fieldId="number-of-accelerators">
<InputGroup>
<NumberInput
inputAriaLabel="Number of accelerators"
id="number-of-accelerators"
name="number-of-accelerators"
value={acceleratorCount}
value={selectedAcceleratorProfile.count}
validated={acceleratorCountWarning ? 'warning' : 'default'}
min={1}
onPlus={() => onStep(1)}
onMinus={() => onStep(-1)}
onChange={(event) => {
if (isHTMLInputElement(event.target)) {
const newSize = Number(event.target.value);
setAcceleratorProfileState('count', Math.max(newSize, 1));
setSelectedAcceleratorProfile('count', Math.max(newSize, 1));
}
}}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const NotebookServerDetails: React.FC = () => {
const { images, loaded } = useWatchImages();
const [isExpanded, setExpanded] = React.useState(false);
const { dashboardConfig } = useAppContext();
const [acceleratorProfile] = useNotebookAcceleratorProfile(notebook);
const acceleratorProfile = useNotebookAcceleratorProfile(notebook);

const container: PodContainer | undefined = notebook?.spec.template.spec.containers.find(
(currentContainer) => currentContainer.name === notebook.metadata.name,
Expand Down Expand Up @@ -112,12 +112,12 @@ const NotebookServerDetails: React.FC = () => {
<DescriptionListDescription>
{acceleratorProfile.acceleratorProfile
? acceleratorProfile.acceleratorProfile.spec.displayName
: acceleratorProfile.useExisting
: acceleratorProfile.unknownProfileDetected
? 'Unknown'
: 'None'}
</DescriptionListDescription>
</DescriptionListGroup>
{!acceleratorProfile.useExisting && (
{!acceleratorProfile.unknownProfileDetected && (
<DescriptionListGroup>
<DescriptionListTerm>Number of accelerators</DescriptionListTerm>
<DescriptionListDescription>{acceleratorProfile.count}</DescriptionListDescription>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,7 @@ const SpawnerPage: React.FC = () => {
tag: undefined,
});
const { selectedSize, setSelectedSize, sizes } = usePreferredNotebookSize();
const [acceleratorProfile, setAcceleratorProfile] =
useNotebookAcceleratorProfile(currentUserNotebook);
const acceleratorProfile = useNotebookAcceleratorProfile(currentUserNotebook);
const [variableRows, setVariableRows] = React.useState<VariableRow[]>([]);
const [submitError, setSubmitError] = React.useState<Error | null>(null);

Expand Down Expand Up @@ -236,10 +235,12 @@ const SpawnerPage: React.FC = () => {
outcome: TrackingOutcome.submit,
accelerator: acceleratorProfile.acceleratorProfile
? `${acceleratorProfile.acceleratorProfile.spec.displayName} (${acceleratorProfile.acceleratorProfile.metadata.name}): ${acceleratorProfile.acceleratorProfile.spec.identifier}`
: acceleratorProfile.useExisting
: acceleratorProfile.unknownProfileDetected
? 'Unknown'
: 'None',
acceleratorCount: acceleratorProfile.useExisting ? undefined : acceleratorProfile.count,
acceleratorCount: acceleratorProfile.unknownProfileDetected
? undefined
: acceleratorProfile.count,
lastSelectedSize: selectedSize.name,
lastSelectedImage: `${selectedImageTag.image?.name}:${selectedImageTag.tag?.name}`,
});
Expand Down Expand Up @@ -320,10 +321,7 @@ const SpawnerPage: React.FC = () => {
setValue={(size) => setSelectedSize(size)}
sizes={sizes}
/>
<AcceleratorProfileSelectField
acceleratorProfileState={acceleratorProfile}
setAcceleratorProfileState={setAcceleratorProfile}
/>
<AcceleratorProfileSelectField acceleratorProfileState={acceleratorProfile} />
</FormSection>
<FormSection title="Environment variables" className="odh-notebook-controller__env-var">
{renderEnvironmentVariableRows()}
Expand Down
8 changes: 5 additions & 3 deletions frontend/src/pages/projects/notebook/NotebookStatusToggle.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const NotebookStatusToggle: React.FC<NotebookStatusToggleProps> = ({
isDisabled,
}) => {
const { notebook, isStarting, isRunning, isStopping, refresh } = notebookState;
const [acceleratorProfile] = useNotebookAcceleratorProfile(notebook);
const acceleratorProfile = useNotebookAcceleratorProfile(notebook);
const { size } = useNotebookDeploymentSize(notebook);
const [isOpenConfirm, setOpenConfirm] = React.useState(false);
const [inProgress, setInProgress] = React.useState(false);
Expand All @@ -55,10 +55,12 @@ const NotebookStatusToggle: React.FC<NotebookStatusToggleProps> = ({
(action: 'started' | 'stopped') => {
fireFormTrackingEvent(`Workbench ${action === 'started' ? 'Started' : 'Stopped'}`, {
outcome: TrackingOutcome.submit,
acceleratorCount: acceleratorProfile.useExisting ? undefined : acceleratorProfile.count,
acceleratorCount: acceleratorProfile.unknownProfileDetected
? undefined
: acceleratorProfile.count,
accelerator: acceleratorProfile.acceleratorProfile
? `${acceleratorProfile.acceleratorProfile.spec.displayName} (${acceleratorProfile.acceleratorProfile.metadata.name}): ${acceleratorProfile.acceleratorProfile.spec.identifier}`
: acceleratorProfile.useExisting
: acceleratorProfile.unknownProfileDetected
? 'Unknown'
: 'None',
lastSelectedSize:
Expand Down
Loading

0 comments on commit c9092cc

Please sign in to comment.