Skip to content

Commit

Permalink
fix(sdk): fix incorrect sub-DAG output type when using dsl.Collected (
Browse files Browse the repository at this point in the history
  • Loading branch information
connor-mccarthy authored Oct 9, 2023
1 parent 0670337 commit fcdff29
Show file tree
Hide file tree
Showing 9 changed files with 66 additions and 40 deletions.
1 change: 1 addition & 0 deletions sdk/RELEASE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
25 changes: 25 additions & 0 deletions sdk/python/kfp/compiler/compiler_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion sdk/python/kfp/compiler/pipeline_spec_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion sdk/python/test_data/pipelines/if_elif_else_complex.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,7 @@ components:
outputDefinitions:
parameters:
pipelinechannel--int-0-to-9999-Output:
parameterType: NUMBER_INTEGER
parameterType: LIST
comp-for-loop-16:
dag:
tasks:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ components:
outputDefinitions:
parameters:
pipelinechannel--double-Output:
parameterType: NUMBER_INTEGER
parameterType: LIST
comp-condition-4:
dag:
outputs:
Expand Down Expand Up @@ -74,7 +74,7 @@ components:
outputDefinitions:
parameters:
pipelinechannel--add-Output:
parameterType: NUMBER_INTEGER
parameterType: LIST
comp-double:
executorLabel: exec-double
inputDefinitions:
Expand Down Expand Up @@ -117,7 +117,7 @@ components:
outputDefinitions:
parameters:
pipelinechannel--double-Output:
parameterType: NUMBER_INTEGER
parameterType: LIST
deploymentSpec:
executors:
exec-add:
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -229,4 +229,4 @@ root:
Output:
parameterType: LIST
schemaVersion: 2.1.0
sdkVersion: kfp-2.1.3
sdkVersion: kfp-2.3.0
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ components:
outputDefinitions:
parameters:
pipelinechannel--add-two-nums-Output:
parameterType: NUMBER_INTEGER
parameterType: LIST
comp-for-loop-4:
dag:
outputs:
Expand Down Expand Up @@ -135,7 +135,7 @@ components:
outputDefinitions:
parameters:
pipelinechannel--add-two-nums-Output:
parameterType: NUMBER_INTEGER
parameterType: LIST
deploymentSpec:
executors:
exec-add:
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -291,4 +291,4 @@ root:
Output:
parameterType: LIST
schemaVersion: 2.1.0
sdkVersion: kfp-2.1.3
sdkVersion: kfp-2.3.0
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -120,7 +120,7 @@ components:
outputDefinitions:
parameters:
pipelinechannel--double-2-Output:
parameterType: NUMBER_INTEGER
parameterType: LIST
comp-for-loop-6:
dag:
outputs:
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -491,4 +491,4 @@ root:
Output:
parameterType: NUMBER_INTEGER
schemaVersion: 2.1.0
sdkVersion: kfp-2.1.3
sdkVersion: kfp-2.3.0
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ components:
outputDefinitions:
parameters:
pipelinechannel--double-Output:
parameterType: NUMBER_INTEGER
parameterType: LIST
deploymentSpec:
executors:
exec-add:
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -184,4 +184,4 @@ root:
Output:
parameterType: LIST
schemaVersion: 2.1.0
sdkVersion: kfp-2.1.3
sdkVersion: kfp-2.3.0
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ components:
outputDefinitions:
parameters:
pipelinechannel--double-pipeline-Output:
parameterType: NUMBER_INTEGER
parameterType: LIST
comp-for-loop-2-2:
dag:
outputs:
Expand Down Expand Up @@ -157,7 +157,7 @@ components:
outputDefinitions:
parameters:
pipelinechannel--echo-and-return-Output:
parameterType: STRING
parameterType: LIST
comp-for-loop-4:
dag:
outputs:
Expand Down Expand Up @@ -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:
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -364,4 +364,4 @@ root:
Output:
parameterType: NUMBER_INTEGER
schemaVersion: 2.1.0
sdkVersion: kfp-2.1.3
sdkVersion: kfp-2.3.0

0 comments on commit fcdff29

Please sign in to comment.