Skip to content

Commit 7efcbc4

Browse files
[raycicmd] Implicit dimension matching for array-to-array depends_on
When array step A depends on array step B via a plain string reference and both share array dimension names (e.g. "python", "cuda"), each expanded variant of A now depends only on the B variant with matching values on those shared dimensions, instead of all variants of B. This unblocks parallelism in the pipeline DAG: py3.11 build no longer waits on py3.10 or py3.12 builds of its dependency. Behavior: - Overlapping dims: constrain to matching values (new) - No overlapping dims: fan out to all variants (unchanged) - Non-array deps: pass through as-is (unchanged) - Explicit selector filter: takes precedence over implicit matching (unchanged) - Opt-out: use map selector form `- step-name:` to force all-variants fanout Topic: implicit-dim-matching Relative: array-validation-guards Labels: draft Signed-off-by: andrew <andrew@anyscale.com>
1 parent 2ca9c57 commit 7efcbc4

3 files changed

Lines changed: 750 additions & 16 deletions

File tree

raycicmd/array_depends.go

Lines changed: 76 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,9 @@ import (
77

88
// arraySelector represents a dependency selector with optional array filter.
99
type arraySelector struct {
10-
key string // step identifier: "key" for command steps, "name" for wanda steps
11-
filter map[string][]string // dimension -> allowed values (nil = all instances)
10+
key string // step identifier
11+
filter map[string][]string // dimension constraints; nil triggers implicit matching or fan-out
12+
matchAll bool // true = depend on all variants (e.g. `- step: all`)
1213
}
1314

1415
// parseArrayDependsOn parses a depends_on field into a list of selectors.
@@ -20,6 +21,8 @@ type arraySelector struct {
2021
// depends_on: # selector with array filter
2122
// - ray-build:
2223
// python: "3.11"
24+
// depends_on: # explicit all-variants
25+
// - ray-build: all
2326
func parseArrayDependsOn(v any) ([]*arraySelector, error) {
2427
if v == nil {
2528
return nil, nil
@@ -73,16 +76,28 @@ func parseArraySelectorMap(m map[string]any) (*arraySelector, error) {
7376

7477
sel := &arraySelector{key: stepName}
7578

76-
if filterVal != nil {
77-
filterMap, ok := filterVal.(map[string]any)
78-
if !ok {
79-
return nil, fmt.Errorf("selector %q filter must be a map, got %T", stepName, filterVal)
79+
switch fv := filterVal.(type) {
80+
case nil:
81+
// `- step-name:` with no value; treat same as plain string.
82+
case string:
83+
if fv != "all" {
84+
return nil, fmt.Errorf(
85+
"selector %q: string value must be \"all\", got %q",
86+
stepName, fv,
87+
)
8088
}
81-
parsed, err := parseSelectorFilter(filterMap)
89+
sel.matchAll = true
90+
case map[string]any:
91+
parsed, err := parseSelectorFilter(fv)
8292
if err != nil {
8393
return nil, fmt.Errorf("in selector for %q: %w", stepName, err)
8494
}
8595
sel.filter = parsed
96+
default:
97+
return nil, fmt.Errorf(
98+
"selector %q filter must be a map or \"all\", got %T",
99+
stepName, filterVal,
100+
)
86101
}
87102

88103
return sel, nil
@@ -108,9 +123,15 @@ func parseSelectorFilter(m map[string]any) (map[string][]string, error) {
108123
}
109124

110125
// resolveDependsOn resolves a depends_on field to concrete step keys.
111-
// Plain references to array steps fan out to all instances.
112-
// Selector references resolve to only matching instances.
113-
func resolveDependsOn(dependsOn any, configs map[string]*arrayConfig) ([]string, error) {
126+
// When currentElem is non-nil and a plain reference targets an array
127+
// step with overlapping dimensions, resolution is constrained to the
128+
// variant matching the current element's values on those dimensions.
129+
// Explicit selector filters take precedence over implicit matching.
130+
func resolveDependsOn(
131+
dependsOn any,
132+
configs map[string]*arrayConfig,
133+
currentElem *arrayElement,
134+
) ([]string, error) {
114135
selectors, err := parseArrayDependsOn(dependsOn)
115136
if err != nil {
116137
return nil, err
@@ -120,7 +141,7 @@ func resolveDependsOn(dependsOn any, configs map[string]*arrayConfig) ([]string,
120141
for _, sel := range selectors {
121142
cfg, isArray := configs[sel.key]
122143
if !isArray {
123-
if sel.filter != nil {
144+
if sel.filter != nil || sel.matchAll {
124145
return nil, fmt.Errorf(
125146
"cannot use array selector on non-array step %q",
126147
sel.key,
@@ -129,8 +150,31 @@ func resolveDependsOn(dependsOn any, configs map[string]*arrayConfig) ([]string,
129150
result = append(result, sel.key)
130151
continue
131152
}
153+
154+
// For plain references (no explicit filter/matchAll) from an
155+
// array step, build an implicit filter on overlapping dims.
156+
// When there is no overlap, sel.filter stays nil and
157+
// resolveArraySelector matches all elements (fan-out).
158+
implicit := false
159+
if sel.filter == nil && !sel.matchAll && currentElem != nil {
160+
if f := implicitDimFilter(currentElem, cfg); f != nil {
161+
sel = &arraySelector{
162+
key: sel.key,
163+
filter: f,
164+
}
165+
implicit = true
166+
}
167+
}
168+
132169
matches, err := resolveArraySelector(sel, cfg)
133170
if err != nil {
171+
if implicit {
172+
return nil, fmt.Errorf(
173+
"implicit dimension matching on %q: %w; "+
174+
"use `- %s: all` to depend on all variants",
175+
sel.key, err, sel.key,
176+
)
177+
}
134178
return nil, err
135179
}
136180
result = append(result, matches...)
@@ -139,6 +183,27 @@ func resolveDependsOn(dependsOn any, configs map[string]*arrayConfig) ([]string,
139183
return result, nil
140184
}
141185

186+
// implicitDimFilter builds a selector filter from the overlapping
187+
// dimensions between the current element and the target config.
188+
// Returns nil if there is no overlap, causing the caller to fall
189+
// through to fan-out-to-all behavior.
190+
func implicitDimFilter(
191+
current *arrayElement, target *arrayConfig,
192+
) map[string][]string {
193+
var filter map[string][]string
194+
for dim := range target.dims {
195+
val, ok := current.values[dim]
196+
if !ok {
197+
continue
198+
}
199+
if filter == nil {
200+
filter = make(map[string][]string)
201+
}
202+
filter[dim] = []string{val}
203+
}
204+
return filter
205+
}
206+
142207
// resolveArraySelector resolves a selector against an array config
143208
// to concrete step keys, using the final element list (post-adjustments).
144209
func resolveArraySelector(sel *arraySelector, cfg *arrayConfig) ([]string, error) {

raycicmd/array_expand.go

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,13 @@ import (
1010
// depends_on references to point to the expanded keys.
1111
func expandArraySteps(gs []*pipelineGroup) error {
1212
configs := make(map[string]*arrayConfig)
13+
// elems maps each expanded step key to its arrayElement,
14+
// used for implicit dimension matching in Pass 2.
15+
elems := make(map[string]*arrayElement)
1316

1417
// Pass 1: expand array steps into resolvedSteps.
1518
for _, g := range gs {
16-
if err := g.buildResolvedSteps(configs); err != nil {
19+
if err := g.buildResolvedSteps(configs, elems); err != nil {
1720
return fmt.Errorf("expand arrays: %w", err)
1821
}
1922
}
@@ -25,8 +28,9 @@ func expandArraySteps(gs []*pipelineGroup) error {
2528
if !ok {
2629
continue
2730
}
31+
currentElem := elems[stepKey(rs.src)]
2832
resolved, err := resolveDependsOn(
29-
dependsOn, configs,
33+
dependsOn, configs, currentElem,
3034
)
3135
if err != nil {
3236
return fmt.Errorf(
@@ -65,6 +69,7 @@ func expandArraySteps(gs []*pipelineGroup) error {
6569
// array steps into multiple entries.
6670
func (g *pipelineGroup) buildResolvedSteps(
6771
configs map[string]*arrayConfig,
72+
elems map[string]*arrayElement,
6873
) error {
6974
var result []*resolvedStep
7075

@@ -95,8 +100,10 @@ func (g *pipelineGroup) buildResolvedSteps(
95100
if err != nil {
96101
return err
97102
}
98-
for _, es := range expanded {
103+
cfg := configs[baseKey]
104+
for j, es := range expanded {
99105
result = append(result, &resolvedStep{src: es})
106+
elems[stepKey(es)] = cfg.elements[j]
100107
}
101108
}
102109

0 commit comments

Comments
 (0)