Skip to content

fix: allocate training resources across all replica sets - #475

Open
shin0343 wants to merge 1 commit into
aws:mainfrom
shin0343:fix/all-replica-resource-allocation
Open

shin0343 wants to merge 1 commit into
aws:mainfrom
shin0343:fix/all-replica-resource-allocation

Conversation

@shin0343

Copy link
Copy Markdown

What's changing and why?

Resolve and validate training resources for every ReplicaSpec, then apply all allocations together. Previously only the first replica was processed, leaving later replicas with unresolved accelerator aliases and no instance-specific resource calculation or validation.

Handle optional resource mappings without suppressing other replicas' allocations. Unmanaged replicas retain their input; managed replicas can use instance defaults. Validation errors propagate before job submission or partial mutation.

Before/After UX

Before: A g5 chief plus p4d workers only allocated the chief; invalid worker accelerator counts bypassed SDK validation. An omitted resource mapping on an additional replica could also suppress the chief's allocation.

After: Each managed replica receives its own CPU/memory/GPU/EFA allocation. Invalid later replicas reject the job before API submission, and optional mappings are supported.

How was this change tested?

  • Full unit suite on Python 3.11: 1,269 passed.
  • 22 new cases cover mixed instance types, unmanaged replicas in both positions, optional/partial mappings, atomic validation failures, absent templates/lists, and public create() payloads/rejection with Kubernetes and telemetry mocked.
  • Changed files parse with Python 3.8 grammar; new tests pass Ruff lint/format; changed files pass git-secrets and git diff --check.
  • Command: PYTHONPATH=src python -m pytest --confcutdir=test/unit_tests -o addopts='' -q test/unit_tests using the prepared environment. The flag avoids the parent template-install hook; default coverage options are separated.

Are unit tests added?

Yes, 22 public allocation and mocked creation cases.

Are integration tests added?

No live AWS/EKS tests. The public creation boundary is exercised with a mocked Kubernetes API.

Reviewer Guidelines

  • All automated PR checks pass
  • Failed tests include local run results/screenshots proving they work
  • Changes are documentation-only

Validate each replica before applying any allocations. Preserve unmanaged optional mappings and reject invalid later replicas before Kubernetes submission.

Signed-off-by: shin0343 <shin0343@mail.hongik.ac.kr>
@shin0343
shin0343 requested a review from a team as a code owner October 25, 2026 16:07
@shin0343

Copy link
Copy Markdown
Author

The current head is f9ca5d4aeba4f1a6a5a57e36478204155fffe16f. collab-check passed; wait-for-approval is still waiting before the external-contributor CI jobs can run.

Could a maintainer approve CI for this head? Local validation recorded in the PR includes 1,269 passing unit tests on Python 3.11 and 22 regression cases covering replica-specific allocation and atomic validation. No live AWS/EKS validation was performed.

This branch is waiting to be deployed

1 waiting deployment
manual-approval — f9ca5d4a Waiting Oct 25, 2026 by shin0343 via wait-for-approval #828
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant