From fcdff294a6323f6cb1c0e574fc7aa5ccc25e420b Mon Sep 17 00:00:00 2001 From: Connor McCarthy Date: Mon, 9 Oct 2023 10:04:18 -0700 Subject: [PATCH] fix(sdk): fix incorrect sub-DAG output type when using `dsl.Collected` (#10069) --- sdk/RELEASE.md | 1 + sdk/python/kfp/compiler/compiler_test.py | 25 ++++++++++++++++++ .../kfp/compiler/pipeline_spec_builder.py | 2 +- .../pipelines/if_elif_else_complex.yaml | 2 +- .../conditional_producer_and_consumers.yaml | 12 ++++----- .../nested_with_parameters.yaml | 14 +++++----- .../parameters_complex.yaml | 26 +++++++++---------- .../parallelfor_fan_in/parameters_simple.yaml | 8 +++--- .../pipeline_producer_consumer.yaml | 16 ++++++------ 9 files changed, 66 insertions(+), 40 deletions(-) diff --git a/sdk/RELEASE.md b/sdk/RELEASE.md index 3f171205cd1..09b20e1b545 100644 --- a/sdk/RELEASE.md +++ b/sdk/RELEASE.md @@ -7,6 +7,7 @@ ## Deprecations ## Bug fixes and other changes +* Fix type on `dsl.ParallelFor` sub-DAG output when a `dsl.Collected` is used. Non-functional fix. [\#10069](https://github.com/kubeflow/pipelines/pull/10069) ## Documentation updates diff --git a/sdk/python/kfp/compiler/compiler_test.py b/sdk/python/kfp/compiler/compiler_test.py index a8b0f37215a..9d2a7cb2deb 100644 --- a/sdk/python/kfp/compiler/compiler_test.py +++ b/sdk/python/kfp/compiler/compiler_test.py @@ -2861,6 +2861,31 @@ def my_pipeline(x: Input[Artifact] = Artifact( class TestCrossTasksGroupFanInCollection(unittest.TestCase): + def test_correct_subdag_return_type(self): + from typing import List + + from kfp import dsl + + @dsl.component + def double(num: int) -> int: + return 2 * num + + @dsl.component + def add(nums: List[int]) -> int: + return sum(nums) + + @dsl.pipeline + def math_pipeline() -> int: + with dsl.ParallelFor([1, 2, 3]) as v: + t = double(num=v) + + return add(nums=dsl.Collected(t.output)).output + + self.assertEqual( + math_pipeline.pipeline_spec.components['comp-for-loop-2'] + .output_definitions.parameters['pipelinechannel--double-Output'] + .parameter_type, type_utils.LIST) + def test_missing_collected_with_correct_annotation(self): from typing import List diff --git a/sdk/python/kfp/compiler/pipeline_spec_builder.py b/sdk/python/kfp/compiler/pipeline_spec_builder.py index e6083d8ba92..1c0b7aa4635 100644 --- a/sdk/python/kfp/compiler/pipeline_spec_builder.py +++ b/sdk/python/kfp/compiler/pipeline_spec_builder.py @@ -626,7 +626,7 @@ def build_component_spec_for_group( else: component_spec.output_definitions.parameters[ output_name].parameter_type = type_utils.get_parameter_type( - channel.channel_type) + output.channel_type) return component_spec diff --git a/sdk/python/test_data/pipelines/if_elif_else_complex.yaml b/sdk/python/test_data/pipelines/if_elif_else_complex.yaml index 0726ea48e00..9f14ee8b69f 100644 --- a/sdk/python/test_data/pipelines/if_elif_else_complex.yaml +++ b/sdk/python/test_data/pipelines/if_elif_else_complex.yaml @@ -439,7 +439,7 @@ components: outputDefinitions: parameters: pipelinechannel--int-0-to-9999-Output: - parameterType: NUMBER_INTEGER + parameterType: LIST comp-for-loop-16: dag: tasks: diff --git a/sdk/python/test_data/pipelines/parallelfor_fan_in/conditional_producer_and_consumers.yaml b/sdk/python/test_data/pipelines/parallelfor_fan_in/conditional_producer_and_consumers.yaml index bf110a3192b..6cf59971e90 100644 --- a/sdk/python/test_data/pipelines/parallelfor_fan_in/conditional_producer_and_consumers.yaml +++ b/sdk/python/test_data/pipelines/parallelfor_fan_in/conditional_producer_and_consumers.yaml @@ -44,7 +44,7 @@ components: outputDefinitions: parameters: pipelinechannel--double-Output: - parameterType: NUMBER_INTEGER + parameterType: LIST comp-condition-4: dag: outputs: @@ -74,7 +74,7 @@ components: outputDefinitions: parameters: pipelinechannel--add-Output: - parameterType: NUMBER_INTEGER + parameterType: LIST comp-double: executorLabel: exec-double inputDefinitions: @@ -117,7 +117,7 @@ components: outputDefinitions: parameters: pipelinechannel--double-Output: - parameterType: NUMBER_INTEGER + parameterType: LIST deploymentSpec: executors: exec-add: @@ -132,7 +132,7 @@ deploymentSpec: - -c - "\nif ! [ -x \"$(command -v pip)\" ]; then\n python3 -m ensurepip ||\ \ python3 -m ensurepip --user || apt-get install python3-pip\nfi\n\nPIP_DISABLE_PIP_VERSION_CHECK=1\ - \ python3 -m pip install --quiet --no-warn-script-location 'kfp==2.1.3'\ + \ python3 -m pip install --quiet --no-warn-script-location 'kfp==2.3.0'\ \ '--no-deps' 'typing-extensions>=3.7.4,<5; python_version<\"3.9\"' && \"\ $0\" \"$@\"\n" - sh @@ -160,7 +160,7 @@ deploymentSpec: - -c - "\nif ! [ -x \"$(command -v pip)\" ]; then\n python3 -m ensurepip ||\ \ python3 -m ensurepip --user || apt-get install python3-pip\nfi\n\nPIP_DISABLE_PIP_VERSION_CHECK=1\ - \ python3 -m pip install --quiet --no-warn-script-location 'kfp==2.1.3'\ + \ python3 -m pip install --quiet --no-warn-script-location 'kfp==2.3.0'\ \ '--no-deps' 'typing-extensions>=3.7.4,<5; python_version<\"3.9\"' && \"\ $0\" \"$@\"\n" - sh @@ -229,4 +229,4 @@ root: Output: parameterType: LIST schemaVersion: 2.1.0 -sdkVersion: kfp-2.1.3 +sdkVersion: kfp-2.3.0 diff --git a/sdk/python/test_data/pipelines/parallelfor_fan_in/nested_with_parameters.yaml b/sdk/python/test_data/pipelines/parallelfor_fan_in/nested_with_parameters.yaml index e8a4ff9021d..ffefb1fac65 100644 --- a/sdk/python/test_data/pipelines/parallelfor_fan_in/nested_with_parameters.yaml +++ b/sdk/python/test_data/pipelines/parallelfor_fan_in/nested_with_parameters.yaml @@ -74,7 +74,7 @@ components: outputDefinitions: parameters: pipelinechannel--add-two-nums-Output: - parameterType: NUMBER_INTEGER + parameterType: LIST comp-for-loop-4: dag: outputs: @@ -135,7 +135,7 @@ components: outputDefinitions: parameters: pipelinechannel--add-two-nums-Output: - parameterType: NUMBER_INTEGER + parameterType: LIST deploymentSpec: executors: exec-add: @@ -150,7 +150,7 @@ deploymentSpec: - -c - "\nif ! [ -x \"$(command -v pip)\" ]; then\n python3 -m ensurepip ||\ \ python3 -m ensurepip --user || apt-get install python3-pip\nfi\n\nPIP_DISABLE_PIP_VERSION_CHECK=1\ - \ python3 -m pip install --quiet --no-warn-script-location 'kfp==2.1.3'\ + \ python3 -m pip install --quiet --no-warn-script-location 'kfp==2.3.0'\ \ '--no-deps' 'typing-extensions>=3.7.4,<5; python_version<\"3.9\"' && \"\ $0\" \"$@\"\n" - sh @@ -179,7 +179,7 @@ deploymentSpec: - -c - "\nif ! [ -x \"$(command -v pip)\" ]; then\n python3 -m ensurepip ||\ \ python3 -m ensurepip --user || apt-get install python3-pip\nfi\n\nPIP_DISABLE_PIP_VERSION_CHECK=1\ - \ python3 -m pip install --quiet --no-warn-script-location 'kfp==2.1.3'\ + \ python3 -m pip install --quiet --no-warn-script-location 'kfp==2.3.0'\ \ '--no-deps' 'typing-extensions>=3.7.4,<5; python_version<\"3.9\"' && \"\ $0\" \"$@\"\n" - sh @@ -207,7 +207,7 @@ deploymentSpec: - -c - "\nif ! [ -x \"$(command -v pip)\" ]; then\n python3 -m ensurepip ||\ \ python3 -m ensurepip --user || apt-get install python3-pip\nfi\n\nPIP_DISABLE_PIP_VERSION_CHECK=1\ - \ python3 -m pip install --quiet --no-warn-script-location 'kfp==2.1.3'\ + \ python3 -m pip install --quiet --no-warn-script-location 'kfp==2.3.0'\ \ '--no-deps' 'typing-extensions>=3.7.4,<5; python_version<\"3.9\"' && \"\ $0\" \"$@\"\n" - sh @@ -235,7 +235,7 @@ deploymentSpec: - -c - "\nif ! [ -x \"$(command -v pip)\" ]; then\n python3 -m ensurepip ||\ \ python3 -m ensurepip --user || apt-get install python3-pip\nfi\n\nPIP_DISABLE_PIP_VERSION_CHECK=1\ - \ python3 -m pip install --quiet --no-warn-script-location 'kfp==2.1.3'\ + \ python3 -m pip install --quiet --no-warn-script-location 'kfp==2.3.0'\ \ '--no-deps' 'typing-extensions>=3.7.4,<5; python_version<\"3.9\"' && \"\ $0\" \"$@\"\n" - sh @@ -291,4 +291,4 @@ root: Output: parameterType: LIST schemaVersion: 2.1.0 -sdkVersion: kfp-2.1.3 +sdkVersion: kfp-2.3.0 diff --git a/sdk/python/test_data/pipelines/parallelfor_fan_in/parameters_complex.yaml b/sdk/python/test_data/pipelines/parallelfor_fan_in/parameters_complex.yaml index efade68e719..9e2c0288af8 100644 --- a/sdk/python/test_data/pipelines/parallelfor_fan_in/parameters_complex.yaml +++ b/sdk/python/test_data/pipelines/parallelfor_fan_in/parameters_complex.yaml @@ -90,9 +90,9 @@ components: outputDefinitions: parameters: pipelinechannel--double-2-Output: - parameterType: NUMBER_INTEGER + parameterType: LIST pipelinechannel--double-Output: - parameterType: NUMBER_INTEGER + parameterType: LIST comp-for-loop-4: dag: outputs: @@ -120,7 +120,7 @@ components: outputDefinitions: parameters: pipelinechannel--double-2-Output: - parameterType: NUMBER_INTEGER + parameterType: LIST comp-for-loop-6: dag: outputs: @@ -167,9 +167,9 @@ components: outputDefinitions: parameters: pipelinechannel--nested-add-2-Output: - parameterType: NUMBER_INTEGER + parameterType: LIST pipelinechannel--simple-add-2-Output: - parameterType: NUMBER_INTEGER + parameterType: LIST comp-nested-add: executorLabel: exec-nested-add inputDefinitions: @@ -224,7 +224,7 @@ deploymentSpec: - -c - "\nif ! [ -x \"$(command -v pip)\" ]; then\n python3 -m ensurepip ||\ \ python3 -m ensurepip --user || apt-get install python3-pip\nfi\n\nPIP_DISABLE_PIP_VERSION_CHECK=1\ - \ python3 -m pip install --quiet --no-warn-script-location 'kfp==2.1.3'\ + \ python3 -m pip install --quiet --no-warn-script-location 'kfp==2.3.0'\ \ '--no-deps' 'typing-extensions>=3.7.4,<5; python_version<\"3.9\"' && \"\ $0\" \"$@\"\n" - sh @@ -253,7 +253,7 @@ deploymentSpec: - -c - "\nif ! [ -x \"$(command -v pip)\" ]; then\n python3 -m ensurepip ||\ \ python3 -m ensurepip --user || apt-get install python3-pip\nfi\n\nPIP_DISABLE_PIP_VERSION_CHECK=1\ - \ python3 -m pip install --quiet --no-warn-script-location 'kfp==2.1.3'\ + \ python3 -m pip install --quiet --no-warn-script-location 'kfp==2.3.0'\ \ '--no-deps' 'typing-extensions>=3.7.4,<5; python_version<\"3.9\"' && \"\ $0\" \"$@\"\n" - sh @@ -281,7 +281,7 @@ deploymentSpec: - -c - "\nif ! [ -x \"$(command -v pip)\" ]; then\n python3 -m ensurepip ||\ \ python3 -m ensurepip --user || apt-get install python3-pip\nfi\n\nPIP_DISABLE_PIP_VERSION_CHECK=1\ - \ python3 -m pip install --quiet --no-warn-script-location 'kfp==2.1.3'\ + \ python3 -m pip install --quiet --no-warn-script-location 'kfp==2.3.0'\ \ '--no-deps' 'typing-extensions>=3.7.4,<5; python_version<\"3.9\"' && \"\ $0\" \"$@\"\n" - sh @@ -309,7 +309,7 @@ deploymentSpec: - -c - "\nif ! [ -x \"$(command -v pip)\" ]; then\n python3 -m ensurepip ||\ \ python3 -m ensurepip --user || apt-get install python3-pip\nfi\n\nPIP_DISABLE_PIP_VERSION_CHECK=1\ - \ python3 -m pip install --quiet --no-warn-script-location 'kfp==2.1.3'\ + \ python3 -m pip install --quiet --no-warn-script-location 'kfp==2.3.0'\ \ '--no-deps' 'typing-extensions>=3.7.4,<5; python_version<\"3.9\"' && \"\ $0\" \"$@\"\n" - sh @@ -338,7 +338,7 @@ deploymentSpec: - -c - "\nif ! [ -x \"$(command -v pip)\" ]; then\n python3 -m ensurepip ||\ \ python3 -m ensurepip --user || apt-get install python3-pip\nfi\n\nPIP_DISABLE_PIP_VERSION_CHECK=1\ - \ python3 -m pip install --quiet --no-warn-script-location 'kfp==2.1.3'\ + \ python3 -m pip install --quiet --no-warn-script-location 'kfp==2.3.0'\ \ '--no-deps' 'typing-extensions>=3.7.4,<5; python_version<\"3.9\"' && \"\ $0\" \"$@\"\n" - sh @@ -367,7 +367,7 @@ deploymentSpec: - -c - "\nif ! [ -x \"$(command -v pip)\" ]; then\n python3 -m ensurepip ||\ \ python3 -m ensurepip --user || apt-get install python3-pip\nfi\n\nPIP_DISABLE_PIP_VERSION_CHECK=1\ - \ python3 -m pip install --quiet --no-warn-script-location 'kfp==2.1.3'\ + \ python3 -m pip install --quiet --no-warn-script-location 'kfp==2.3.0'\ \ '--no-deps' 'typing-extensions>=3.7.4,<5; python_version<\"3.9\"' && \"\ $0\" \"$@\"\n" - sh @@ -395,7 +395,7 @@ deploymentSpec: - -c - "\nif ! [ -x \"$(command -v pip)\" ]; then\n python3 -m ensurepip ||\ \ python3 -m ensurepip --user || apt-get install python3-pip\nfi\n\nPIP_DISABLE_PIP_VERSION_CHECK=1\ - \ python3 -m pip install --quiet --no-warn-script-location 'kfp==2.1.3'\ + \ python3 -m pip install --quiet --no-warn-script-location 'kfp==2.3.0'\ \ '--no-deps' 'typing-extensions>=3.7.4,<5; python_version<\"3.9\"' && \"\ $0\" \"$@\"\n" - sh @@ -491,4 +491,4 @@ root: Output: parameterType: NUMBER_INTEGER schemaVersion: 2.1.0 -sdkVersion: kfp-2.1.3 +sdkVersion: kfp-2.3.0 diff --git a/sdk/python/test_data/pipelines/parallelfor_fan_in/parameters_simple.yaml b/sdk/python/test_data/pipelines/parallelfor_fan_in/parameters_simple.yaml index e537b147b8b..c315ffee16f 100644 --- a/sdk/python/test_data/pipelines/parallelfor_fan_in/parameters_simple.yaml +++ b/sdk/python/test_data/pipelines/parallelfor_fan_in/parameters_simple.yaml @@ -60,7 +60,7 @@ components: outputDefinitions: parameters: pipelinechannel--double-Output: - parameterType: NUMBER_INTEGER + parameterType: LIST deploymentSpec: executors: exec-add: @@ -75,7 +75,7 @@ deploymentSpec: - -c - "\nif ! [ -x \"$(command -v pip)\" ]; then\n python3 -m ensurepip ||\ \ python3 -m ensurepip --user || apt-get install python3-pip\nfi\n\nPIP_DISABLE_PIP_VERSION_CHECK=1\ - \ python3 -m pip install --quiet --no-warn-script-location 'kfp==2.1.3'\ + \ python3 -m pip install --quiet --no-warn-script-location 'kfp==2.3.0'\ \ '--no-deps' 'typing-extensions>=3.7.4,<5; python_version<\"3.9\"' && \"\ $0\" \"$@\"\n" - sh @@ -113,7 +113,7 @@ deploymentSpec: - -c - "\nif ! [ -x \"$(command -v pip)\" ]; then\n python3 -m ensurepip ||\ \ python3 -m ensurepip --user || apt-get install python3-pip\nfi\n\nPIP_DISABLE_PIP_VERSION_CHECK=1\ - \ python3 -m pip install --quiet --no-warn-script-location 'kfp==2.1.3'\ + \ python3 -m pip install --quiet --no-warn-script-location 'kfp==2.3.0'\ \ '--no-deps' 'typing-extensions>=3.7.4,<5; python_version<\"3.9\"' && \"\ $0\" \"$@\"\n" - sh @@ -184,4 +184,4 @@ root: Output: parameterType: LIST schemaVersion: 2.1.0 -sdkVersion: kfp-2.1.3 +sdkVersion: kfp-2.3.0 diff --git a/sdk/python/test_data/pipelines/parallelfor_fan_in/pipeline_producer_consumer.yaml b/sdk/python/test_data/pipelines/parallelfor_fan_in/pipeline_producer_consumer.yaml index dc9c8b9adad..2c6e4e46123 100644 --- a/sdk/python/test_data/pipelines/parallelfor_fan_in/pipeline_producer_consumer.yaml +++ b/sdk/python/test_data/pipelines/parallelfor_fan_in/pipeline_producer_consumer.yaml @@ -129,7 +129,7 @@ components: outputDefinitions: parameters: pipelinechannel--double-pipeline-Output: - parameterType: NUMBER_INTEGER + parameterType: LIST comp-for-loop-2-2: dag: outputs: @@ -157,7 +157,7 @@ components: outputDefinitions: parameters: pipelinechannel--echo-and-return-Output: - parameterType: STRING + parameterType: LIST comp-for-loop-4: dag: outputs: @@ -185,7 +185,7 @@ components: outputDefinitions: parameters: pipelinechannel--double-pipeline-Output: - parameterType: NUMBER_INTEGER + parameterType: LIST comp-join-and-print: executorLabel: exec-join-and-print inputDefinitions: @@ -206,7 +206,7 @@ deploymentSpec: - -c - "\nif ! [ -x \"$(command -v pip)\" ]; then\n python3 -m ensurepip ||\ \ python3 -m ensurepip --user || apt-get install python3-pip\nfi\n\nPIP_DISABLE_PIP_VERSION_CHECK=1\ - \ python3 -m pip install --quiet --no-warn-script-location 'kfp==2.1.3'\ + \ python3 -m pip install --quiet --no-warn-script-location 'kfp==2.3.0'\ \ '--no-deps' 'typing-extensions>=3.7.4,<5; python_version<\"3.9\"' && \"\ $0\" \"$@\"\n" - sh @@ -235,7 +235,7 @@ deploymentSpec: - -c - "\nif ! [ -x \"$(command -v pip)\" ]; then\n python3 -m ensurepip ||\ \ python3 -m ensurepip --user || apt-get install python3-pip\nfi\n\nPIP_DISABLE_PIP_VERSION_CHECK=1\ - \ python3 -m pip install --quiet --no-warn-script-location 'kfp==2.1.3'\ + \ python3 -m pip install --quiet --no-warn-script-location 'kfp==2.3.0'\ \ '--no-deps' 'typing-extensions>=3.7.4,<5; python_version<\"3.9\"' && \"\ $0\" \"$@\"\n" - sh @@ -263,7 +263,7 @@ deploymentSpec: - -c - "\nif ! [ -x \"$(command -v pip)\" ]; then\n python3 -m ensurepip ||\ \ python3 -m ensurepip --user || apt-get install python3-pip\nfi\n\nPIP_DISABLE_PIP_VERSION_CHECK=1\ - \ python3 -m pip install --quiet --no-warn-script-location 'kfp==2.1.3'\ + \ python3 -m pip install --quiet --no-warn-script-location 'kfp==2.3.0'\ \ '--no-deps' 'typing-extensions>=3.7.4,<5; python_version<\"3.9\"' && \"\ $0\" \"$@\"\n" - sh @@ -292,7 +292,7 @@ deploymentSpec: - -c - "\nif ! [ -x \"$(command -v pip)\" ]; then\n python3 -m ensurepip ||\ \ python3 -m ensurepip --user || apt-get install python3-pip\nfi\n\nPIP_DISABLE_PIP_VERSION_CHECK=1\ - \ python3 -m pip install --quiet --no-warn-script-location 'kfp==2.1.3'\ + \ python3 -m pip install --quiet --no-warn-script-location 'kfp==2.3.0'\ \ '--no-deps' 'typing-extensions>=3.7.4,<5; python_version<\"3.9\"' && \"\ $0\" \"$@\"\n" - sh @@ -364,4 +364,4 @@ root: Output: parameterType: NUMBER_INTEGER schemaVersion: 2.1.0 -sdkVersion: kfp-2.1.3 +sdkVersion: kfp-2.3.0