Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Make SDG batch size configurable via system profile (backport #3157)#3208

Merged
ktdreyer merged 1 commit intorelease-v0.23instructlab/instructlab:release-v0.23from
mergify/bp/release-v0.23/pr-3157instructlab/instructlab:mergify/bp/release-v0.23/pr-3157Copy head branch name to clipboard
Mar 3, 2025
Merged

Make SDG batch size configurable via system profile (backport #3157)#3208
ktdreyer merged 1 commit intorelease-v0.23instructlab/instructlab:release-v0.23from
mergify/bp/release-v0.23/pr-3157instructlab/instructlab:mergify/bp/release-v0.23/pr-3157Copy head branch name to clipboard

Conversation

@mergify
Copy link
Contributor

@mergify mergify bot commented Feb 28, 2025

Currently, the batch size for SDG is only configurable via the CLI, but a single batch size across all hardware profiles is not optimal. Different hardware configurations have varying capabilities, and using a fixed batch size can lead to under-utilization or over-utilization of resources during the SDG process.

To ensure efficient performance across different hardware, we should set the batch sizes independently in each system profile.

This is an automatic backport of pull request #3157 done by Mergify.

@npalaska @mergify
make SDG batch size configurable via system profile
bb2a7b6 
Signed-off-by: Nikhil Palaskar <npalaska@redhat.com>
(cherry picked from commit 4a309ce)
@mergify mergify bot mentioned this pull request Feb 28, 2025
Merged
@mergify mergify bot added testing Relates to testing release-branch Pull Request directly to a release branch ci-failure PR has at least one CI failure and removed ci-failure PR has at least one CI failure labels Feb 28, 2025
@courtneypacheco courtneypacheco added the hold In-progress PR. Tag should be removed before merge. label Mar 3, 2025
@courtneypacheco
Copy link
Contributor

courtneypacheco commented Mar 3, 2025

There is a failure in the large E2E job on main: https://github.com/instructlab/instructlab/actions/runs/13623238238/job/38076244547

I'm going to run the large E2E job on this branch to ensure no conflicts on this particular release branch.

@github-actions
Copy link

github-actions bot commented Mar 3, 2025

E2E (NVIDIA L40S x4) workflow launched on this PR: View run

@github-actions
Copy link

github-actions bot commented Mar 3, 2025

e2e workflow failed on this PR: View run, please investigate.

@courtneypacheco
Copy link
Contributor

courtneypacheco commented Mar 3, 2025

The above job failed due to a download failure with HuggingFace. Rerunning.

@github-actions
Copy link

github-actions bot commented Mar 3, 2025

E2E (NVIDIA L40S x4) workflow launched on this PR: View run

@github-actions
Copy link

github-actions bot commented Mar 3, 2025

e2e workflow failed on this PR: View run, please investigate.

@courtneypacheco
Copy link
Contributor

courtneypacheco commented Mar 3, 2025

The same HuggingFace download error occurred. It seems like a server-side error, so I will trigger the job once more.

@github-actions
Copy link

github-actions bot commented Mar 3, 2025

E2E (NVIDIA L40S x4) workflow launched on this PR: View run

@mergify mergify bot removed the ci-failure PR has at least one CI failure label Mar 3, 2025
@github-actions
Copy link

github-actions bot commented Mar 3, 2025

e2e workflow succeeded on this PR: View run, congrats!

@mergify mergify bot added the ci-failure PR has at least one CI failure label Mar 3, 2025
@ktdreyer
Copy link
Contributor

ktdreyer commented Mar 3, 2025

Unit tests fail on this PR. Known failure, and we fixed this for release-v0.23 in #3210.

We're tracking the HuggingFace download error here: #3215 .

@mergify mergify bot added the one-approval PR has one approval from a maintainer label Mar 3, 2025
@courtneypacheco courtneypacheco removed the hold In-progress PR. Tag should be removed before merge. label Mar 3, 2025
@mergify mergify bot removed the one-approval PR has one approval from a maintainer label Mar 3, 2025
@ktdreyer
Copy link
Contributor

ktdreyer commented Mar 3, 2025

Courtney & I discussed this today in a call.

Even though we fixed the unit tests in #3210, this PR does not reflect that change until we rebase on that. In the interest of time, we will not manually rebase this PR. We need to ship the upcoming v0.23.3 release today, and this is a low-risk change.

Summary: We will merge this PR over the failing unit tests. After it is merged, we'll verify that unit tests continue to pass on the release-v0.23 branch.

@ktdreyer ktdreyer merged commit 728a226 into release-v0.23 Mar 3, 2025
23 of 27 checks passed
@ktdreyer ktdreyer deleted the mergify/bp/release-v0.23/pr-3157 branch March 3, 2025 16:53
@ktdreyer ktdreyer mentioned this pull request Mar 3, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@ktdreyer ktdreyer ktdreyer approved these changes

@courtneypacheco courtneypacheco courtneypacheco approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

ci-failure PR has at least one CI failure release-branch Pull Request directly to a release branch testing Relates to testing

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@courtneypacheco @ktdreyer @npalaska

Comments

Close sidebar
Morty Proxy This is a proxified and sanitized view of the page, visit original site.