fix(inference): break circular import in ops.transformer.inference - #8465
fix(inference): break circular import in ops.transformer.inference#8465chakshu-dhannawat wants to merge 2 commits into
Conversation
Values <= 0 for max_out_tokens were accepted during config parsing and only failed at generation time with a confusing 'input size exceeds maximum length of -1' error. Add a field validator so DeepSpeedInferenceConfig raises a clear ValueError immediately. Signed-off-by: Chakshu Dhannawat <chakshu.dhannawat1@gmail.com>
Importing deepspeed.ops.transformer.inference eagerly loaded DeepSpeedTransformerInference from model_implementations.transformers.ds_transformer. That module imports Triton kernels at load time, and importing any submodule under deepspeed.ops.transformer.inference.triton while the parent inference package was still initializing caused a circular-import error on systems where Triton is installed. Changes: - Replace the eager import with a PEP-562 __getattr__ and an explicit __all__, so DeepSpeedTransformerInference is resolved only when accessed. - Add a CPU-only regression test asserting the symbol stays reachable. Closes deepspeedai#7159 Signed-off-by: Chakshu Dhannawat <chakshu.dhannawat1@gmail.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d76362e009
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| from ....model_implementations.transformers.ds_transformer import DeepSpeedTransformerInference | ||
| from .moe_inference import DeepSpeedMoEInferenceConfig, DeepSpeedMoEInference | ||
|
|
||
| __all__ = [ |
There was a problem hiding this comment.
This non-merge commit has no Signed-off-by trailer in its commit metadata. Add the required sign-off using the configured author identity before merging.
AGENTS.md reference: AGENTS.md:L8-L8
Useful? React with 👍 / 👎.
| return field_value | ||
| raise TypeError(f"Invalid type for dtype: {type(field_value)}") | ||
|
|
||
| @field_validator("max_out_tokens") |
There was a problem hiding this comment.
The commit records pre-commit run --files only for __init__.py and test_inference_import.py, but it also changes deepspeed/inference/config.py and test_inference_config.py. Run the required pre-commit command over all four modified files before merging.
AGENTS.md reference: AGENTS.md:L10-L10
Useful? React with 👍 / 👎.
1 participant
Closes #7159
Importing
deepspeed.ops.transformer.inferenceeagerly loadedDeepSpeedTransformerInferencefrommodel_implementations.transformers.ds_transformer. That module imports Triton kernels at load time, and importing any submodule underdeepspeed.ops.transformer.inference.tritonwhile the parentinferencepackage was still initializing caused a circular-import error on systems where Triton is installed.Changes:
__getattr__and an explicit__all__, soDeepSpeedTransformerInferenceis resolved only when accessed.Verified:
python -c "import deepspeed; from deepspeed.ops.transformer.inference import DeepSpeedTransformerInference"passes withdeepspeed.HAS_TRITON=True.pytest tests/unit/inference/test_inference_import.py -m inference -xvspasses.pre-commit run --files deepspeed/ops/transformer/inference/__init__.py tests/unit/inference/test_inference_import.pypasses.