Skip to content

Commit

Permalink
Back porting lambda arn bug, HPO validation bug, and bumping to 1.5.1
Browse files Browse the repository at this point in the history
  • Loading branch information
estohlmann authored Jun 26, 2024
1 parent 8bb9827 commit 56500f7
Show file tree
Hide file tree
Showing 6 changed files with 19,202 additions and 97 deletions.
2 changes: 2 additions & 0 deletions bin/mlspace-cdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,8 @@ const coreStack = new CoreStack(app, 'mlspace-core', {
encryptionKey: kmsStack.masterKey,
mlSpaceAppRole,
mlSpaceNotebookRole,
mlspaceEndpointConfigInstanceConstraintPolicy,
mlspaceJobInstanceConstraintPolicy,
mlSpaceVPC,
lambdaSecurityGroups: [vpcStack.vpcSecurityGroup],
mlSpaceDefaultSecurityGroupId: vpcStack.vpcSecurityGroupId,
Expand Down
19,269 changes: 19,184 additions & 85 deletions frontend/package-lock.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion frontend/package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "@amzn/mlspace",
"homepage": "/Prod",
"version": "1.5.0",
"version": "1.5.1",
"main": "dist/lib/index.js",
"types": "dist/types/index.d.ts",
"private": true,
Expand Down
1 change: 1 addition & 0 deletions frontend/src/entities/jobs/hpo/create/hpo-job-create.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,7 @@ export function HPOJobCreate () {
element={
<Wizard
activeStepIndex={state.activeStepIndex}
isLoadingNextStep={state.formSubmitting}
onNavigate={(event) => {
switch (event.detail.reason) {
case 'step':
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,6 @@ export function TrainingJobDefinition (props: TrainingJobDefinitionProps) {
}
});
const parameterRanges = Object.values(item.HyperParameterRanges.ContinuousParameterRanges).concat(Object.values(item.HyperParameterRanges.IntegerParameterRanges));

parameterRanges.forEach(
(parameterRange: any) => {
const hyperParameter = algorithm.defaultHyperParameters.find(
Expand All @@ -243,14 +242,9 @@ export function TrainingJobDefinition (props: TrainingJobDefinitionProps) {
let nanError = false;
[parameterRange.MinValue, parameterRange.MaxValue].forEach(
(value, index) => {
// Only evaluate the validator if:
// - The field is required
// - OR the field has a value
// - OR this is the max field and the min field has a value
// This evaluation avoids the issue where a blank value is evaluated as a 0 on a forced safeParse even for optional fields
if ((!hyperParameter.zValidator?.isOptional || value || (index === 1 && parameterRange.MinValue)) && !nanError ) {
if (hyperParameter.zValidator && !nanError ) {
// Any word alternatives should not apply to ranges
let parseResult = hyperParameter.zValidator?.safeParse(value);
let parseResult = hyperParameter.zValidator.safeParse(value);
if (parseResult?.success === false) {
ctx.addIssue({
code: 'custom',
Expand All @@ -264,6 +258,13 @@ export function TrainingJobDefinition (props: TrainingJobDefinitionProps) {
) +
` (${index === 0 ? 'min value' : 'max value'})`,
});
} else if (value && ((index === 1 && !parameterRange.MinValue) || (index === 0 && !parameterRange.MaxValue))) {
// Ensure the min/max fields are populated if their counterpart is populated
ctx.addIssue({
code: 'custom',
path: ['hyperparameters', parameterRange.Name],
message: `Please set a ${index === 1 ? 'min ' : 'max'} value`,
});
} else {
// Check if the user put an approved non-number alternate value in the range field and alert on this unique scenario
parseResult = z.coerce.number({
Expand Down
8 changes: 5 additions & 3 deletions lib/stacks/infra/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { ISecurityGroup, IVpc } from 'aws-cdk-lib/aws-ec2';
import { CfnSecurityConfiguration } from 'aws-cdk-lib/aws-emr';
import { Rule, Schedule } from 'aws-cdk-lib/aws-events';
import { LambdaFunction } from 'aws-cdk-lib/aws-events-targets';
import { Effect, IRole, PolicyStatement, Role, ServicePrincipal } from 'aws-cdk-lib/aws-iam';
import { Effect, IManagedPolicy, IRole, PolicyStatement, Role, ServicePrincipal } from 'aws-cdk-lib/aws-iam';
import { IKey } from 'aws-cdk-lib/aws-kms';
import { Code, Function } from 'aws-cdk-lib/aws-lambda';
import {
Expand Down Expand Up @@ -53,6 +53,8 @@ export type CoreStackProps = {
readonly encryptionKey: IKey;
readonly mlSpaceAppRole: IRole;
readonly mlSpaceNotebookRole: IRole;
readonly mlspaceEndpointConfigInstanceConstraintPolicy?: IManagedPolicy,
readonly mlspaceJobInstanceConstraintPolicy?: IManagedPolicy,
readonly mlSpaceVPC: IVpc;
readonly mlSpaceDefaultSecurityGroupId: string;
readonly isIso?: boolean;
Expand Down Expand Up @@ -237,8 +239,8 @@ export class CoreStack extends Stack {
role: props.mlSpaceAppRole,
environment: {
APP_CONFIG_TABLE: props.mlspaceConfig.APP_CONFIGURATION_TABLE_NAME,
ENDPOINT_CONFIG_INSTANCE_CONSTRAINT_POLICY_ARN: props.mlspaceConfig.ENDPOINT_CONFIG_INSTANCE_CONSTRAINT_POLICY_ARN,
JOB_INSTANCE_CONSTRAINT_POLICY_ARN: props.mlspaceConfig.JOB_INSTANCE_CONSTRAINT_POLICY_ARN,
ENDPOINT_CONFIG_INSTANCE_CONSTRAINT_POLICY_ARN: props.mlspaceEndpointConfigInstanceConstraintPolicy?.managedPolicyArn || '',
JOB_INSTANCE_CONSTRAINT_POLICY_ARN: props.mlspaceJobInstanceConstraintPolicy?.managedPolicyArn || '',
SYSTEM_TAG: props.mlspaceConfig.SYSTEM_TAG,
MANAGE_IAM_ROLES: props.mlspaceConfig.MANAGE_IAM_ROLES ? 'True' : '',
},
Expand Down

0 comments on commit 56500f7

Please sign in to comment.