已开启
CP modeling: unify device_per_node and bw_intra/bw_inter data sources (follow-up PR) #306
zhangyuguo创建于  7月27日
zhangyuguo
7月27日 创建

Follow-up from R29's review on the CP modeling PR. The three items below are deferred to separate fix/bug/feature PRs — they require deeper refactoring of the Device type and level_bandwidth semantics, which is out of scope for the current CP modeling review. Filing this issue to track them and address them together once the Device/level_bandwidth structure is fully understood.


1. Unify device_per_node source

device_per_node in cp_comm_layer_detailed and related paths should be derived from machine.device.intra_node_num() instead of the flat field on ccfg.

2. Recalibrate bw_intra / bw_inter dimensionality

ccfg.bw_intra = 400 and Device_A2.level_bandwidth[0] = 50 are not the same quantity:

  • level_bandwidth = [50, 10] is per-link physical bandwidth (introduced by ch-l in 0f0e617a1 / 738c37f5a, the Type abstraction treats it as per-link).
  • ccfg.bw_intra = 400 is the effective/aggregate bandwidth after 8-card HCCS aggregation (calibrated value).

Directly substituting one for the other would inflate intra-node CP communication time by ~8x and distort the search-space ranking. The level_bandwidth semantics (per-link vs aggregate) must be recalibrated before deciding whether to replace ccfg.bw_intra/bw_inter with values from level_bandwidth.

3. Consolidate the three bandwidth sources into a single source of truth

Currently there are three independent bandwidth sources:

  • Production path: cp_comm_layer_detailed → _cp_resolve_topology reads ccfg.bw_intra / ccfg.bw_inter (flat fields, defaults 400 / 25).
  • Test path: get_cp_topology hardcodes 300 / 25.
  • hardware.py: Device_A2.level_bandwidth = [50, 10].

Unifying these three into a single source of truth has impact beyond the CP modeling review scope, so it will be handled as a separate PR.


Dependency: Item (1) is a prerequisite for (2) and (3) — the device_per_node source must be unified first before the bandwidth dimensionality can be recalibrated and the sources consolidated. The three items will be addressed together in one follow-up PR.

likedislike
zhangyuguo
7月27日 评论:

Reviewer pointer from R30 (for whoever picks up this PR):

hardware.py:271 already has get_cp_bandwidth(topology_type, device_type) — it reads device.level_bandwidth[0] / [1] (per-link values from the Type abstraction). Per R30 review, this function is currently unused on the production path (only referenced in tests/ut/auto_parallel/sapp_nd/test_cp_modeling.py bandwidth tests), so it is both:

  • a partial implementation item (3) can build on — the per-link → aggregate dispatch logic already exists,
  • a cleanup target — once the single source of truth is settled, this function becomes the natural API surface for item (3) consolidation.

Suggested approach for item (3): make get_cp_bandwidth the single entry point, have it return the aggregate (8-card HCCS) value, and route both cp_comm_layer_detailed (currently ccfg.bw_intra / ccfg.bw_inter) and the test path (currently hardcoded 300 / 25) through it.

likedislike