Skip to content

Commit f16a414

Browse files
mgornymgorny
committed
Do not combine non-adjacent conditions in v1 recipes
Modify the v1 recipe condition combiner not to merge non-adjacent conditions. While this is not really a problem for dependencies (though it could break original dependency grouping), it did lead to reordering test commands that could lead to breaking the recipe. The algorithm aims to be as simple as possible. It iterates over lists tail-to-head and folds an "if" into the preceding item if it has the same condition. Fixes regro#3933
1 parent 1d7012a commit f16a414

File tree

3 files changed

+85
-63
lines changed

3 files changed

+85
-63
lines changed

‎conda_forge_tick/migrators/recipe_v1.py‎

Lines changed: 34 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -6,62 +6,51 @@
66
from conda_forge_tick.migrators.core import MiniMigrator
77
from conda_forge_tick.recipe_parser._parser import _get_yaml_parser
88

9+
from typing import Any
10+
911
if typing.TYPE_CHECKING:
1012
from ..migrators_types import AttrsTypedDict
1113

1214
logger = logging.getLogger(__name__)
1315

1416

15-
def combine_conditions(node):
17+
def is_same_condition(a: Any, b: Any) -> bool:
18+
return (
19+
isinstance(a, dict)
20+
and isinstance(b, dict)
21+
and "if" in a
22+
and "if" in b
23+
and a["if"] == b["if"]
24+
)
25+
26+
27+
def fold_branch(source: Any, dest: Any, branch: str) -> None:
28+
if branch not in source:
29+
return
30+
source_l = source[branch]
31+
if isinstance(source_l, str):
32+
source_l = [source_l]
33+
34+
if branch not in dest:
35+
dest[branch] = []
36+
elif isinstance(dest[branch], str):
37+
dest[branch] = [dest[branch]]
38+
dest[branch].extend(source_l)
39+
40+
41+
def combine_conditions(node: Any):
1642
"""Breadth first recursive call to combine list conditions"""
1743

1844
# recursion is breadth first because we go through each element here
1945
# before calling `combine_conditions` on any element in the node
2046
if isinstance(node, list):
21-
# 1. loop through list elements, gather the if conditions
22-
23-
# condition ("if:") -> [(then, else), (then, else)...]
24-
conditions = {}
25-
26-
for i in node:
27-
if isinstance(i, dict) and "if" in i:
28-
conditions.setdefault(i["if"], []).append((i["then"], i.get("else")))
29-
30-
# 2. if elements share a compatible if condition
31-
# combine their if...then...else statements
32-
to_drop = []
33-
for i in range(len(node)):
34-
if isinstance(node[i], dict) and "if" in node[i]:
35-
condition = node[i]["if"]
36-
if condition not in conditions:
37-
# already combined it, so drop the repeat instance
38-
to_drop.append(i)
39-
continue
40-
if len(conditions[condition]) > 1:
41-
new_then = []
42-
new_else = []
43-
for sub_then, sub_else in conditions[node[i]["if"]]:
44-
if isinstance(sub_then, list):
45-
new_then.extend(sub_then)
46-
else:
47-
assert sub_then is not None
48-
new_then.append(sub_then)
49-
if isinstance(sub_else, list):
50-
new_else.extend(sub_else)
51-
elif sub_else is not None:
52-
new_else.append(sub_else)
53-
node[i]["then"] = new_then
54-
if new_else:
55-
# TODO: preserve inline "else" instead of converting it to a list?
56-
node[i]["else"] = new_else
57-
else:
58-
assert "else" not in node[i]
59-
# remove it from the dict, so we don't output it again
60-
del conditions[condition]
61-
62-
# drop the repeated conditions
63-
for i in reversed(to_drop):
64-
del node[i]
47+
# iterate in reverse order, so we can remove elements on the fly
48+
# start at index 1, since we can only fold to the previous node
49+
for i in reversed(range(1, len(node))):
50+
if is_same_condition(node[i], node[i - 1]):
51+
fold_branch(node[i], node[i - 1], "then")
52+
fold_branch(node[i], node[i - 1], "else")
53+
del node[i]
6554

6655
# then we descend down the tree
6756
if isinstance(node, dict):

‎tests/test_v1_yaml/version_pytorch.yaml‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,8 @@ outputs:
318318
- torch
319319
- torch._C
320320
- files:
321+
recipe:
322+
- cmake_test/
321323
source:
322324
- test
323325
- tools
@@ -369,6 +371,15 @@ outputs:
369371
then: test ! -f $SP_DIR/functorch/__pycache__/__init__.cpython-313.pyc
370372
- if: match(python, "!=3.13") and win
371373
then: if exist %SP_DIR%\functorch\__pycache__\__init__.cpython-313.pyc exit 1
374+
- cd cmake_test
375+
- if: unix
376+
then: cmake -GNinja -DCMAKE_CXX_STANDARD=17 -DWITH_TORCH_PYTHON=ON $CMAKE_ARGS .
377+
- if: win
378+
then: cmake -GNinja -DCMAKE_CXX_STANDARD=17 -DWITH_TORCH_PYTHON=ON %CMAKE_ARGS% .
379+
- if: unix
380+
then: cmake --build .
381+
- if: win
382+
then: cmake --build . --config Release
372383

373384
about:
374385
license: BSD-3-Clause

‎tests/test_v1_yaml/version_pytorch_correct.yaml‎

Lines changed: 40 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@ outputs:
5555
then:
5656
- intel-openmp ${{ mkl }}
5757
- libuv
58-
- sccache
5958
- cmake
6059
- ninja
6160
# Keep libprotobuf here so that a compatibile version
@@ -64,18 +63,27 @@ outputs:
6463
- protobuf
6564
- if: linux
6665
then: make
66+
- if: win
67+
then: sccache
6768
- if: unix
6869
then:
6970
- grep
7071
- rsync
7172
host:
7273
# GPU requirements
74+
- if: "cuda_compiler_version != \"None\""
75+
then: cudnn
76+
- if: "cuda_compiler_version != \"None\" and linux"
77+
then: nccl
7378
- if: "cuda_compiler_version != \"None\""
7479
then:
75-
- cudnn
7680
- magma
7781
- cuda-version ${{ cuda_compiler_version }}
7882
- nvtx-c
83+
- if: "linux and cuda_compiler_version != \"None\""
84+
then: cuda-driver-dev
85+
- if: "cuda_compiler_version != \"None\""
86+
then:
7987
- cuda-cudart-dev
8088
- cuda-cupti-dev
8189
- cuda-nvrtc-dev
@@ -85,16 +93,14 @@ outputs:
8593
- cusparselt
8694
- libcublas-dev
8795
- libcudss-dev
96+
- if: "linux and cuda_compiler_version != \"None\""
97+
then: libcufile-dev
98+
- if: "cuda_compiler_version != \"None\""
99+
then:
88100
- libcufft-dev
89101
- libcurand-dev
90102
- libcusolver-dev
91103
- libcusparse-dev
92-
- if: "cuda_compiler_version != \"None\" and linux"
93-
then: nccl
94-
- if: "linux and cuda_compiler_version != \"None\""
95-
then:
96-
- cuda-driver-dev
97-
- libcufile-dev
98104
- if: megabuild
99105
else:
100106
- python
@@ -181,12 +187,19 @@ outputs:
181187
host:
182188
- ${{ pin_subpackage('libtorch', exact=True) }}
183189
# GPU requirements
190+
- if: "cuda_compiler_version != \"None\""
191+
then: cudnn
192+
- if: "cuda_compiler_version != \"None\" and linux"
193+
then: nccl
184194
- if: "cuda_compiler_version != \"None\""
185195
then:
186-
- cudnn
187196
- cuda-version ${{ cuda_compiler_version }}
188197
- nvtx-c
189198
- magma
199+
- if: "linux and cuda_compiler_version != \"None\""
200+
then: cuda-driver-dev
201+
- if: "cuda_compiler_version != \"None\""
202+
then:
190203
- cuda-cudart-dev
191204
- cuda-cupti-dev
192205
- cuda-nvrtc-dev
@@ -196,16 +209,14 @@ outputs:
196209
- cusparselt
197210
- libcublas-dev
198211
- libcudss-dev
212+
- if: "linux and cuda_compiler_version != \"None\""
213+
then: libcufile-dev
214+
- if: "cuda_compiler_version != \"None\""
215+
then:
199216
- libcufft-dev
200217
- libcurand-dev
201218
- libcusolver-dev
202219
- libcusparse-dev
203-
- if: "cuda_compiler_version != \"None\" and linux"
204-
then: nccl
205-
- if: "linux and cuda_compiler_version != \"None\""
206-
then:
207-
- cuda-driver-dev
208-
- libcufile-dev
209220
- python
210221
- numpy
211222
- pip
@@ -252,12 +263,12 @@ outputs:
252263
then: nomkl
253264
# GPU requirements without run_exports
254265
- if: "cuda_compiler_version != \"None\""
255-
then:
256-
- ${{ pin_compatible('cudnn') }}
257-
- __cuda
266+
then: ${{ pin_compatible('cudnn') }}
258267
- if: "cuda_compiler_version != \"None\" and not win"
259268
then: triton =${{ triton }}
260269
# avoid that people without GPUs needlessly download ~0.5-1GB
270+
- if: "cuda_compiler_version != \"None\""
271+
then: __cuda
261272
- python
262273
# other requirements, see https://github.com/pytorch/pytorch/blame/main/requirements.txt
263274
- filelock
@@ -285,6 +296,8 @@ outputs:
285296
- torch
286297
- torch._C
287298
- files:
299+
recipe:
300+
- cmake_test/
288301
source:
289302
- test
290303
- tools
@@ -333,6 +346,15 @@ outputs:
333346
then: test ! -f $SP_DIR/functorch/__pycache__/__init__.cpython-313.pyc
334347
- if: match(python, "!=3.13") and win
335348
then: if exist %SP_DIR%\functorch\__pycache__\__init__.cpython-313.pyc exit 1
349+
- cd cmake_test
350+
- if: unix
351+
then: cmake -GNinja -DCMAKE_CXX_STANDARD=17 -DWITH_TORCH_PYTHON=ON $CMAKE_ARGS .
352+
- if: win
353+
then: cmake -GNinja -DCMAKE_CXX_STANDARD=17 -DWITH_TORCH_PYTHON=ON %CMAKE_ARGS% .
354+
- if: unix
355+
then: cmake --build .
356+
- if: win
357+
then: cmake --build . --config Release
336358

337359
about:
338360
license: BSD-3-Clause

0 commit comments

Comments
 (0)