[core] Rename label_domain to topology strategy in scheduling policy#64384
[core] Rename label_domain to topology strategy in scheduling policy#64384tanmayrauth wants to merge 3 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request renames "label domain" scheduling concepts to "topology" or "topology-aware" scheduling across the Ray scheduler codebase, including policy classes, strategy enums, scheduling options, build targets, and associated tests. The feedback points out an ineffective std::move on a const structured binding key in topology_bundle_scheduling_policy.cc that should be simplified to a direct copy.
| result.selected_topology_assignment = | ||
| std::make_pair(label_key, std::move(topology_value)); |
There was a problem hiding this comment.
In C++17, structured bindings for map keys (such as topology_value from topology_groups) are deduced as const std::string. Applying std::move to a const object has no effect and silently falls back to a copy, which can be misleading. Removing std::move makes it clear that a copy is occurring.
| result.selected_topology_assignment = | |
| std::make_pair(label_key, std::move(topology_value)); | |
| result.selected_topology_assignment = | |
| std::make_pair(label_key, topology_value); |
The placement-group topology API was migrated to the topology_strategy
naming convention everywhere except the raylet scheduling-policy layer,
which still used the old label_domain naming.
Rename the remaining label_domain/LabelDomain identifiers to match the
topology convention already used in the proto, GCS, and Python layers:
- LabelDomainSchedulingStrategy -> TopologySchedulingStrategy
- LabelDomain{,StrictPack}SchedulingPolicy{Interface} -> Topology...
- target_label_domain_ -> target_topology_assignment_
- selected_label_domain -> selected_topology_assignment
- label_domain_scheduling_strategy_ -> topology_scheduling_strategy_
- label_domain_bundle_scheduling_policy.{cc,h} -> topology_bundle_...
Pure rename; no behavior change. Resolves the leftover TODO(ray-project#64370).
Closes ray-project#64370
Signed-off-by: Tanmay Rauth <t_rauth@apple.com>
tanmayrauth
force-pushed
the
rename-label-domain-to-topology-strategy
branch
from
d354b8b to
4ae4c06
Compare
|
@abrarsheikh can you please review it? |
|
This is still a bit confusing, we're using "topology" or "topology assignment" or "topology-aware". Let's just standardize to "topology-strategy". |
ray-gardener
Bot
added
core
Address review feedback on the label_domain -> topology rename: - Standardize the inconsistent "topology-aware" wording onto the "topology strategy" vocabulary used by the topology_strategy proto field and Python API, including pre-existing mentions so nothing desyncs. Keep "topology assignment" distinct, as it mirrors the merged Get/Set/ClearTopologyAssignment GCS API and proto field. - Drop the no-op std::move on the const structured-binding key in TopologyStrictPackSchedulingPolicy::Schedule (copy is what happens anyway). No behavior change. Signed-off-by: Tanmay Rauth <t_rauth@apple.com>
abrarsheikh
left a comment
abrarsheikh
left a comment
There was a problem hiding this comment.
please fix the linter issue. I recommend setting up https://docs.ray.io/en/latest/ray-contribute/development.html#pre-commit-hooks
Signed-off-by: Tanmay Rauth <t_rauth@apple.com>
3 participants
The placement-group topology API was migrated to the topology_strategy naming convention everywhere except the raylet scheduling-policy layer, which still used the old label_domain naming.
Rename the remaining label_domain/LabelDomain identifiers to match the topology convention already used in the proto, GCS, and Python layers:
Pure rename; no behavior change.
Closes #64370