feat(launcher): auto-set OMP_NUM_THREADS based on cpus_per_task#859
Conversation
Summary of ChangesHello @rchardx, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances performance and resource management by implementing dynamic control over thread-related environment variables. By automatically configuring settings like OMP_NUM_THREADS based on the number of allocated CPU cores, the changes aim to mitigate issues like thread explosion in multi-process training. This ensures more efficient utilization of computational resources and a more stable execution environment across different deployment launchers. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
The pull request introduces a valuable feature to dynamically manage thread control environment variables, preventing thread explosion in multi-process training. The implementation correctly prioritizes user-defined environment variables and provides a sensible fallback. The update of the default cpu value in SchedulingSpec and its corresponding documentation is also well-aligned with the goal of better resource utilization. However, there is a significant opportunity to improve code maintainability by refactoring duplicated logic related to extracting scheduling specifications and environment variables across the launcher files.
There was a problem hiding this comment.
Pull request overview
This PR introduces automatic configuration of thread control environment variables (OMP_NUM_THREADS, MKL_NUM_THREADS, etc.) to prevent thread explosion in multi-process training scenarios. However, there are critical bugs in how the thread counts are calculated across different launchers.
Changes:
- Added
get_thread_env_vars()utility function to compute thread environment variables with priority-based override support - Changed
SchedulingSpec.cpudefault from 4 to 8 to match the new default thread count - Integrated thread environment variable setting in Local, Ray, and Slurm launchers for both training and inference workloads
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| areal/utils/launcher.py | Added get_thread_env_vars() function to compute thread control environment variables based on CPU allocation |
| areal/api/cli_args.py | Updated SchedulingSpec.cpu default value from 4 to 8 |
| docs/cli_reference.md | Updated documentation to reflect new default CPU value of 8 |
| areal/launcher/slurm.py | Integrated thread environment variables for trainer and LLM servers (contains bugs) |
| areal/launcher/ray.py | Integrated thread environment variables for trainer and LLM servers (contains bugs) |
| areal/launcher/local.py | Integrated thread environment variables for trainer and LLM servers (contains bugs) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
5665111 to
d766066
Compare
garrett4wade
left a comment
There was a problem hiding this comment.
THREAD_ENV_VARS has not been set for controllers. We should also set them in @areal/scheduler/
Dynamically compute thread control environment variables (OMP_NUM_THREADS, MKL_NUM_THREADS, etc.) based on allocated CPU cores to prevent thread explosion in multi-process training scenarios. Changes: - Add get_thread_env_vars() function in launcher utils - Update SchedulingSpec.cpu default from 4 to 8 - Apply thread env vars in Local, Ray, and Slurm launchers - Support user override via SchedulingSpec.env_vars or os.environ
d766066 to
c07b914
Compare
garrett4wade
left a comment
There was a problem hiding this comment.
LGTM. Waiting for CI.
…l-project#859) Dynamically compute thread control environment variables (OMP_NUM_THREADS, MKL_NUM_THREADS, etc.) based on allocated CPU cores to prevent thread explosion in multi-process training scenarios. Changes: - Add get_thread_env_vars() function in launcher utils - Update SchedulingSpec.cpu default from 4 to 8 - Apply thread env vars in Local, Ray, and Slurm launchers - Support user override via SchedulingSpec.env_vars or os.environ
…l-project#859) Dynamically compute thread control environment variables (OMP_NUM_THREADS, MKL_NUM_THREADS, etc.) based on allocated CPU cores to prevent thread explosion in multi-process training scenarios. Changes: - Add get_thread_env_vars() function in launcher utils - Update SchedulingSpec.cpu default from 4 to 8 - Apply thread env vars in Local, Ray, and Slurm launchers - Support user override via SchedulingSpec.env_vars or os.environ
Description
Dynamically compute thread control environment variables (OMP_NUM_THREADS, MKL_NUM_THREADS, etc.) based on allocated CPU cores to prevent thread explosion in multi-process training scenarios.
Changes:
Type of Change
work as expected)
Checklist
jb build docs/gemini review)