Skip to content
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions vllm_ascend/eplb/core/policy/policy_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from .policy_abstract import DynamicConfig, EplbPolicy
from .policy_dynamic_ep import DynamicEplb
from .policy_dynamic_ep_v2 import DynamicEplbV2
from .policy_flashlb import FlashLB
from .policy_flashlb import FlashLB, warm_up
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

This import of warm_up supports a fix that is logically flawed. The recommended approach is to make warm_up a method of the FlashLB class, which would make this import unnecessary and lead to a more correct and maintainable solution. This would involve reverting this change and modifying policy_flashlb.py.

Suggested change
from .policy_flashlb import FlashLB, warm_up
from .policy_flashlb import FlashLB

from .policy_random import RandomLoadBalance


Expand All @@ -29,5 +29,5 @@ def generate_policy(policy_type: int, config: DynamicConfig) -> EplbPolicy:
policy_class = policy.get(policy_type, RandomLoadBalance)
policy_instance = policy_class(config)
if policy_type == 3:
policy_instance.warm_up()
return policy_instance
warm_up()
return policy_instance
Comment on lines +32 to +33
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

While this change fixes the AttributeError, it introduces a significant logical issue. The warm_up() function being called is a module-level function that creates its own separate FlashLB instance with a hardcoded configuration. Consequently, the policy_instance that was just created is not the one being warmed up.

This approach is misleading and potentially incorrect. The warm-up uses a hardcoded configuration that might differ from the runtime configuration of policy_instance, which could lead to suboptimal performance or JIT recompilation.

The original code (policy_instance.warm_up()) suggests the correct intent. The fix should be in vllm_ascend/eplb/core/policy/policy_flashlb.py by making warm_up a method of the FlashLB class. This would ensure the correct instance is warmed up with its own configuration. I recommend reverting the changes in this file and applying the fix in policy_flashlb.py instead.

Suggested change
warm_up()
return policy_instance
policy_instance.warm_up()
return policy_instance

Loading