Repository navigation
Handle missing bandwidth in SM estimation #713
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -508,8 +508,19 @@ def get_expected_topk(num_groups: int) -> float: | |
| nvlink_traffic += self.num_nvlink_ranks / self.num_ranks * (1 - 1 / self.num_nvlink_ranks) # Except local bypass | ||
| rdma_traffic += (self.num_ranks - self.num_nvlink_ranks) / self.num_ranks | ||
|
|
||
| # Found the bounded one | ||
| if self.num_scaleout_ranks > 1 and (rdma_traffic / rdma_gbs) > (nvlink_traffic / nvlink_gbs): | ||
| # Find the bounded link. Detection helpers return 0 on failure, so treat | ||
| # a link with traffic but unknown bandwidth as unbounded transfer time | ||
| # instead of dividing by zero. | ||
| def get_transfer_time(traffic: float, bandwidth: float) -> float: | ||
| if traffic == 0: | ||
| return 0.0 | ||
| return traffic / bandwidth if bandwidth > 0 else math.inf | ||
|
|
||
| rdma_time = get_transfer_time(rdma_traffic, rdma_gbs) | ||
| nvlink_time = get_transfer_time(nvlink_traffic, nvlink_gbs) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 suggestion: 当两条链路 transfer time 均为 math.inf 时, 🤖 v5 |
||
| # If both are unknown, either branch selects zero bandwidth and the | ||
| # calculation below conservatively retains the full device SM count. | ||
| if self.num_scaleout_ranks > 1 and rdma_time > nvlink_time: | ||
| bounded_traffic, bounded_gbs = rdma_traffic, rdma_gbs | ||
| else: | ||
| bounded_traffic, bounded_gbs = nvlink_traffic, nvlink_gbs | ||
|
|
@@ -518,7 +529,7 @@ def get_expected_topk(num_groups: int) -> float: | |
| # NOTES: will try to use more SMs if not overlap with compute | ||
| num_device_sms = torch.cuda.get_device_properties('cuda').multi_processor_count | ||
| num_sms = num_device_sms # No traffic, e.g., EP=1 | ||
| if bounded_traffic > 0: | ||
| if bounded_traffic > 0 and bounded_gbs > 0: | ||
| num_sms = max( | ||
| # Dispatch | ||
| bounded_gbs / bounded_traffic * sm_read / sm_read_gbs, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,97 @@ | ||
| import importlib.util | ||
| import math | ||
| import sys | ||
| import types | ||
| import unittest | ||
| from pathlib import Path | ||
| from unittest.mock import patch | ||
|
|
||
| import torch | ||
|
|
||
|
|
||
| def add_module(name, **attributes): | ||
| module = types.ModuleType(name) | ||
| for key, value in attributes.items(): | ||
| setattr(module, key, value) | ||
| sys.modules[name] = module | ||
| return module | ||
|
|
||
|
|
||
| def load_elastic(): | ||
| module_names = ( | ||
| 'deep_ep', | ||
| 'deep_ep.buffers', | ||
| 'deep_ep._C', | ||
| 'deep_ep.utils', | ||
| 'deep_ep.utils.event', | ||
| 'deep_ep.utils.math', | ||
| 'deep_ep.utils.semantic', | ||
| 'deep_ep.utils.envs', | ||
| 'deep_ep.utils.comm', | ||
| ) | ||
| saved_modules = {name: sys.modules.get(name) for name in module_names} | ||
| try: | ||
| package = add_module('deep_ep') | ||
| package.__path__ = [] | ||
| buffers = add_module('deep_ep.buffers') | ||
| buffers.__path__ = [] | ||
| utils = add_module('deep_ep.utils') | ||
| utils.__path__ = [] | ||
|
|
||
| add_module('deep_ep._C', EventHandle=type('EventHandle', (), {})) | ||
| add_module('deep_ep.utils.event', EventOverlap=type('EventOverlap', (), {})) | ||
| add_module('deep_ep.utils.math', align=lambda x, y: math.ceil(x / y) * y) | ||
| add_module( | ||
| 'deep_ep.utils.semantic', | ||
| value_or=lambda value, default: default if value is None else value, | ||
| weak_lru=lambda *args, **kwargs: lambda function: function, | ||
| ) | ||
| add_module( | ||
| 'deep_ep.utils.envs', | ||
| check_fast_rdma_atomic_support=lambda: True, | ||
| check_nvlink_connections=lambda group: None, | ||
| check_torch_deterministic=lambda: None, | ||
| get_nvlink_gbs=lambda: 0, | ||
| get_rdma_gbs=lambda: 0, | ||
| ) | ||
| add_module('deep_ep.utils.comm', get_nccl_comm_handle=lambda *args, **kwargs: None) | ||
|
|
||
| path = Path(__file__).parents[2] / 'deep_ep' / 'buffers' / 'ep.py' | ||
| spec = importlib.util.spec_from_file_location('deep_ep.buffers.ep', path) | ||
| module = importlib.util.module_from_spec(spec) | ||
| spec.loader.exec_module(module) | ||
| return module | ||
| finally: | ||
| for name, saved_module in saved_modules.items(): | ||
| if saved_module is None: | ||
| sys.modules.pop(name, None) | ||
| else: | ||
| sys.modules[name] = saved_module | ||
|
|
||
|
|
||
| def make_hybrid_buffer(elastic): | ||
| """Build the minimal EPBuffer topology used by the host-only test.""" | ||
| buffer = elastic.EPBuffer.__new__(elastic.EPBuffer) | ||
| buffer.num_rdma_ranks = 2 | ||
| buffer.num_nvlink_ranks = 8 | ||
| buffer.num_scaleout_ranks = 2 | ||
| buffer.num_scaleup_ranks = 8 | ||
| buffer.num_ranks = 16 | ||
| buffer.prefer_overlap_with_compute = True | ||
| return buffer | ||
|
|
||
|
|
||
| class TheoreticalSMTests(unittest.TestCase): | ||
| def test_missing_hybrid_bandwidth_falls_back_to_device_sm_count(self): | ||
| elastic = load_elastic() | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 suggestion: 测试通过 ElasticBuffer.new 绕过 init 并手工设置 num_rdma_ranks、num_nvlink_ranks、num_scaleout_ranks、num_scaleup_ranks、num_ranks、prefer_overlap_with_compute 等内部属性,与实现字段存在隐式耦合:若未来重命名这些属性,测试会以 AttributeError 而非断言失败的形式报错。建议在测试方法 docstring 中注明所依赖的属性契约,或抽出一个小工厂函数集中维护。非阻塞。 🤖 v5 |
||
| buffer = make_hybrid_buffer(elastic) | ||
|
|
||
| properties = types.SimpleNamespace(multi_processor_count=132) | ||
| with patch.object(torch.cuda, 'get_device_properties', return_value=properties): | ||
| num_sms = buffer.get_theoretical_num_sms(64, 8) | ||
|
|
||
| self.assertEqual(num_sms, 132) | ||
|
|
||
|
|
||
| if __name__ == '__main__': | ||
| unittest.main() | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔵 suggestion: get_transfer_time 在 traffic == 0 时返回 int 0,其余分支返回 float;建议改为返回 0.0,与类型注解
-> float保持一致。纯风格问题,不影响行为。🤖 v5