kubeflow
/
pipelines
mirror of https://github.com/kubeflow/pipelines.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

SDK - Made outputs with original names available in ContainerOp.outputs (#3734) 

* SDK - Made outputs with original names available in ContainerOp.outputs

Previously, ContainerOp had strict requirements for the output names, so we had to convert all the names before passing them to the ContainerOp constructor. Outputs with non-pythonic names could not be accessed using their original names.
Now ContainerOp supports any output names, so we're now using the original output names.
However to support legacy pipelines, we're also adding output references with pythonic names.

* Fixed the compiler test data

* Fixed the duplicate parameter outputs in the compiled workflow

* Fixed long line

* Stabilized the output naming conflict resolution

* Fix case of missing special outputs
This commit is contained in:
Alexey Volkov 2020-05-12 19:08:26 -07:00 committed by GitHub
parent fe30d5462a
commit 8ba366b03f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 98 additions and 76 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
sdk/python/kfp/compiler/_op_to_template.py
View File

@ -155,7 +155,7 @@ def _outputs_to_json(op: BaseOp,
else: else:
value_from_key = "path" value_from_key = "path"
output_parameters = [] output_parameters = []
for param in outputs.values(): for param in set(outputs.values()): # set() dedupes output references
output_parameters.append({ output_parameters.append({
'name': param.full_name, 'name': param.full_name,
'valueFrom': { 'valueFrom': {

27
sdk/python/kfp/dsl/_component_bridge.py
View File

@ -40,11 +40,6 @@ def _create_container_op_from_component_and_arguments(
arguments=arguments, arguments=arguments,
) )
#Renaming outputs to conform with ContainerOp/Argo
output_names = (resolved_cmd.output_paths or {}).keys()
output_name_to_python = generate_unique_name_conversion_table(output_names, _sanitize_python_function_name)
output_paths_for_container_op = {output_name_to_python[name]: path for name, path in resolved_cmd.output_paths.items()}
container_spec = component_spec.implementation.container container_spec = component_spec.implementation.container
task = dsl.ContainerOp( task = dsl.ContainerOp(
@ -52,7 +47,7 @@ def _create_container_op_from_component_and_arguments(
image=container_spec.image, image=container_spec.image,
command=resolved_cmd.command, command=resolved_cmd.command,
arguments=resolved_cmd.args, arguments=resolved_cmd.args,
file_outputs=output_paths_for_container_op, file_outputs=resolved_cmd.output_paths,
artifact_argument_paths=[ artifact_argument_paths=[
dsl.InputArgumentPath( dsl.InputArgumentPath(
argument=arguments[input_name], argument=arguments[input_name],
@ -62,18 +57,26 @@ def _create_container_op_from_component_and_arguments(
for input_name, path in resolved_cmd.input_paths.items() for input_name, path in resolved_cmd.input_paths.items()
], ],
) )
# Fixing ContainerOp output types
if component_spec.outputs:
for output in component_spec.outputs:
pythonic_name = output_name_to_python[output.name]
if pythonic_name in task.outputs:
task.outputs[pythonic_name].param_type = output.type
component_meta = copy.copy(component_spec) component_meta = copy.copy(component_spec)
component_meta.implementation = None component_meta.implementation = None
task._set_metadata(component_meta) task._set_metadata(component_meta)
task._component_ref = component_ref task._component_ref = component_ref
# Previously, ContainerOp had strict requirements for the output names, so we had to
# convert all the names before passing them to the ContainerOp constructor.
# Outputs with non-pythonic names could not be accessed using their original names.
# Now ContainerOp supports any output names, so we're now using the original output names.
# However to support legacy pipelines, we're also adding output references with pythonic names.
# TODO: Add warning when people use the legacy output names.
output_names = [output_spec.name for output_spec in component_spec.outputs or []] # Stabilizing the ordering
output_name_to_python = generate_unique_name_conversion_table(output_names, _sanitize_python_function_name)
for output_name in output_names:
pythonic_output_name = output_name_to_python[output_name]
# Note: Some component outputs are currently missing from task.outputs (e.g. MLPipeline UI Metadata)
if pythonic_output_name not in task.outputs and output_name in task.outputs:
task.outputs[pythonic_output_name] = task.outputs[output_name]
if container_spec.env: if container_spec.env:
from kubernetes import client as k8s_client from kubernetes import client as k8s_client
for name, value in container_spec.env.items(): for name, value in container_spec.env.items():

13
sdk/python/kfp/dsl/_container_op.py
View File

@ -1098,9 +1098,13 @@ class ContainerOp(BaseOp):
name: _pipeline_param.PipelineParam(name, op_name=self.name) name: _pipeline_param.PipelineParam(name, op_name=self.name)
for name in file_outputs.keys() for name in file_outputs.keys()
} }
if len(self.outputs) == 1: # Syntactic sugar: Add task.output attribute if the component has a single output.
self.output = list(self.outputs.values())[0] # TODO: Currently the "MLPipeline UI Metadata" output is removed from outputs to preserve backwards compatibility.
# Maybe stop excluding it from outputs, but rather exclude it from unique_outputs.
unique_outputs = set(self.outputs.values())
if len(unique_outputs) == 1:
self.output = list(unique_outputs)[0]
else: else:
self.output = _MultipleOutputsError() self.output = _MultipleOutputsError()
@ -1164,9 +1168,6 @@ class ContainerOp(BaseOp):
output_type = output_meta.type output_type = output_meta.type
self.outputs[output].param_type = output_type self.outputs[output].param_type = output_type
if len(self.outputs) == 1:
self.output = list(self.outputs.values())[0]
def add_pvolumes(self, def add_pvolumes(self,
pvolumes: Dict[str, V1Volume] = None): pvolumes: Dict[str, V1Volume] = None):
"""Updates the existing pvolumes dict, extends volumes and volume_mounts """Updates the existing pvolumes dict, extends volumes and volume_mounts

114
sdk/python/tests/compiler/testdata/parallelfor_item_argument_resolving.yaml vendored
View File

@ -14,7 +14,7 @@ spec:
container: container:
args: args:
- "--param1" - "--param1"
- "{{inputs.parameters.produce-list-of-strings-output}}" - "{{inputs.parameters.produce-list-of-strings-Output}}"
command: command:
- python3 - python3
- "-u" - "-u"
@ -47,7 +47,7 @@ spec:
inputs: inputs:
parameters: parameters:
- -
name: produce-list-of-strings-output name: produce-list-of-strings-Output
metadata: metadata:
labels: labels:
pipelines.kubeflow.org/pipeline-sdk-type: kfp pipelines.kubeflow.org/pipeline-sdk-type: kfp
@ -58,7 +58,7 @@ spec:
container: container:
args: args:
- "--param1" - "--param1"
- "{{inputs.parameters.produce-list-of-strings-output-loop-item}}" - "{{inputs.parameters.produce-list-of-strings-Output-loop-item}}"
command: command:
- python3 - python3
- "-u" - "-u"
@ -91,7 +91,7 @@ spec:
inputs: inputs:
parameters: parameters:
- -
name: produce-list-of-strings-output-loop-item name: produce-list-of-strings-Output-loop-item
metadata: metadata:
labels: labels:
pipelines.kubeflow.org/pipeline-sdk-type: kfp pipelines.kubeflow.org/pipeline-sdk-type: kfp
@ -102,7 +102,7 @@ spec:
container: container:
args: args:
- "--param1" - "--param1"
- "{{inputs.parameters.produce-str-output}}" - "{{inputs.parameters.produce-str-Output}}"
command: command:
- python3 - python3
- "-u" - "-u"
@ -135,7 +135,7 @@ spec:
inputs: inputs:
parameters: parameters:
- -
name: produce-str-output name: produce-str-Output
metadata: metadata:
labels: labels:
pipelines.kubeflow.org/pipeline-sdk-type: kfp pipelines.kubeflow.org/pipeline-sdk-type: kfp
@ -146,7 +146,7 @@ spec:
container: container:
args: args:
- "--param1" - "--param1"
- "{{inputs.parameters.produce-list-of-ints-output}}" - "{{inputs.parameters.produce-list-of-ints-Output}}"
command: command:
- python3 - python3
- "-u" - "-u"
@ -179,7 +179,7 @@ spec:
inputs: inputs:
parameters: parameters:
- -
name: produce-list-of-ints-output name: produce-list-of-ints-Output
metadata: metadata:
labels: labels:
pipelines.kubeflow.org/pipeline-sdk-type: kfp pipelines.kubeflow.org/pipeline-sdk-type: kfp
@ -190,7 +190,7 @@ spec:
container: container:
args: args:
- "--param1" - "--param1"
- "{{inputs.parameters.produce-list-of-ints-output-loop-item}}" - "{{inputs.parameters.produce-list-of-ints-Output-loop-item}}"
command: command:
- python3 - python3
- "-u" - "-u"
@ -223,7 +223,7 @@ spec:
inputs: inputs:
parameters: parameters:
- -
name: produce-list-of-ints-output-loop-item name: produce-list-of-ints-Output-loop-item
metadata: metadata:
labels: labels:
pipelines.kubeflow.org/pipeline-sdk-type: kfp pipelines.kubeflow.org/pipeline-sdk-type: kfp
@ -234,7 +234,7 @@ spec:
container: container:
args: args:
- "--param1" - "--param1"
- "{{inputs.parameters.produce-list-of-dicts-output}}" - "{{inputs.parameters.produce-list-of-dicts-Output}}"
command: command:
- python3 - python3
- "-u" - "-u"
@ -267,7 +267,7 @@ spec:
inputs: inputs:
parameters: parameters:
- -
name: produce-list-of-dicts-output name: produce-list-of-dicts-Output
metadata: metadata:
labels: labels:
pipelines.kubeflow.org/pipeline-sdk-type: kfp pipelines.kubeflow.org/pipeline-sdk-type: kfp
@ -278,7 +278,7 @@ spec:
container: container:
args: args:
- "--param1" - "--param1"
- "{{inputs.parameters.produce-list-of-dicts-output-loop-item-subvar-aaa}}" - "{{inputs.parameters.produce-list-of-dicts-Output-loop-item-subvar-aaa}}"
command: command:
- python3 - python3
- "-u" - "-u"
@ -311,7 +311,7 @@ spec:
inputs: inputs:
parameters: parameters:
- -
name: produce-list-of-dicts-output-loop-item-subvar-aaa name: produce-list-of-dicts-Output-loop-item-subvar-aaa
metadata: metadata:
labels: labels:
pipelines.kubeflow.org/pipeline-sdk-type: kfp pipelines.kubeflow.org/pipeline-sdk-type: kfp
@ -325,34 +325,34 @@ spec:
arguments: arguments:
parameters: parameters:
- -
name: produce-list-of-strings-output name: produce-list-of-strings-Output
value: "{{inputs.parameters.produce-list-of-strings-output}}" value: "{{inputs.parameters.produce-list-of-strings-Output}}"
name: consume name: consume
template: consume template: consume
- -
arguments: arguments:
parameters: parameters:
- -
name: produce-list-of-strings-output-loop-item name: produce-list-of-strings-Output-loop-item
value: "{{inputs.parameters.produce-list-of-strings-output-loop-item}}" value: "{{inputs.parameters.produce-list-of-strings-Output-loop-item}}"
name: consume-2 name: consume-2
template: consume-2 template: consume-2
- -
arguments: arguments:
parameters: parameters:
- -
name: produce-str-output name: produce-str-Output
value: "{{inputs.parameters.produce-str-output}}" value: "{{inputs.parameters.produce-str-Output}}"
name: consume-3 name: consume-3
template: consume-3 template: consume-3
inputs: inputs:
parameters: parameters:
- -
name: produce-list-of-strings-output name: produce-list-of-strings-Output
- -
name: produce-list-of-strings-output-loop-item name: produce-list-of-strings-Output-loop-item
- -
name: produce-str-output name: produce-str-Output
name: for-loop-for-loop-00000001-1 name: for-loop-for-loop-00000001-1
- -
dag: dag:
@ -361,24 +361,24 @@ spec:
arguments: arguments:
parameters: parameters:
- -
name: produce-list-of-ints-output name: produce-list-of-ints-Output
value: "{{inputs.parameters.produce-list-of-ints-output}}" value: "{{inputs.parameters.produce-list-of-ints-Output}}"
name: consume-4 name: consume-4
template: consume-4 template: consume-4
- -
arguments: arguments:
parameters: parameters:
- -
name: produce-list-of-ints-output-loop-item name: produce-list-of-ints-Output-loop-item
value: "{{inputs.parameters.produce-list-of-ints-output-loop-item}}" value: "{{inputs.parameters.produce-list-of-ints-Output-loop-item}}"
name: consume-5 name: consume-5
template: consume-5 template: consume-5
inputs: inputs:
parameters: parameters:
- -
name: produce-list-of-ints-output name: produce-list-of-ints-Output
- -
name: produce-list-of-ints-output-loop-item name: produce-list-of-ints-Output-loop-item
name: for-loop-for-loop-00000002-2 name: for-loop-for-loop-00000002-2
- -
dag: dag:
@ -387,24 +387,24 @@ spec:
arguments: arguments:
parameters: parameters:
- -
name: produce-list-of-dicts-output name: produce-list-of-dicts-Output
value: "{{inputs.parameters.produce-list-of-dicts-output}}" value: "{{inputs.parameters.produce-list-of-dicts-Output}}"
name: consume-6 name: consume-6
template: consume-6 template: consume-6
- -
arguments: arguments:
parameters: parameters:
- -
name: produce-list-of-dicts-output-loop-item-subvar-aaa name: produce-list-of-dicts-Output-loop-item-subvar-aaa
value: "{{inputs.parameters.produce-list-of-dicts-output-loop-item-subvar-aaa}}" value: "{{inputs.parameters.produce-list-of-dicts-Output-loop-item-subvar-aaa}}"
name: consume-7 name: consume-7
template: consume-7 template: consume-7
inputs: inputs:
parameters: parameters:
- -
name: produce-list-of-dicts-output name: produce-list-of-dicts-Output
- -
name: produce-list-of-dicts-output-loop-item-subvar-aaa name: produce-list-of-dicts-Output-loop-item-subvar-aaa
name: for-loop-for-loop-00000003-3 name: for-loop-for-loop-00000003-3
- -
dag: dag:
@ -413,48 +413,48 @@ spec:
arguments: arguments:
parameters: parameters:
- -
name: produce-list-of-strings-output name: produce-list-of-strings-Output
value: "{{tasks.produce-list-of-strings.outputs.parameters.produce-list-of-strings-output}}" value: "{{tasks.produce-list-of-strings.outputs.parameters.produce-list-of-strings-Output}}"
- -
name: produce-list-of-strings-output-loop-item name: produce-list-of-strings-Output-loop-item
value: "{{item}}" value: "{{item}}"
- -
name: produce-str-output name: produce-str-Output
value: "{{tasks.produce-str.outputs.parameters.produce-str-output}}" value: "{{tasks.produce-str.outputs.parameters.produce-str-Output}}"
dependencies: dependencies:
- produce-list-of-strings - produce-list-of-strings
- produce-str - produce-str
name: for-loop-for-loop-00000001-1 name: for-loop-for-loop-00000001-1
template: for-loop-for-loop-00000001-1 template: for-loop-for-loop-00000001-1
withParam: "{{tasks.produce-list-of-strings.outputs.parameters.produce-list-of-strings-output}}" withParam: "{{tasks.produce-list-of-strings.outputs.parameters.produce-list-of-strings-Output}}"
- -
arguments: arguments:
parameters: parameters:
- -
name: produce-list-of-ints-output name: produce-list-of-ints-Output
value: "{{tasks.produce-list-of-ints.outputs.parameters.produce-list-of-ints-output}}" value: "{{tasks.produce-list-of-ints.outputs.parameters.produce-list-of-ints-Output}}"
- -
name: produce-list-of-ints-output-loop-item name: produce-list-of-ints-Output-loop-item
value: "{{item}}" value: "{{item}}"
dependencies: dependencies:
- produce-list-of-ints - produce-list-of-ints
name: for-loop-for-loop-00000002-2 name: for-loop-for-loop-00000002-2
template: for-loop-for-loop-00000002-2 template: for-loop-for-loop-00000002-2
withParam: "{{tasks.produce-list-of-ints.outputs.parameters.produce-list-of-ints-output}}" withParam: "{{tasks.produce-list-of-ints.outputs.parameters.produce-list-of-ints-Output}}"
- -
arguments: arguments:
parameters: parameters:
- -
name: produce-list-of-dicts-output name: produce-list-of-dicts-Output
value: "{{tasks.produce-list-of-dicts.outputs.parameters.produce-list-of-dicts-output}}" value: "{{tasks.produce-list-of-dicts.outputs.parameters.produce-list-of-dicts-Output}}"
- -
name: produce-list-of-dicts-output-loop-item-subvar-aaa name: produce-list-of-dicts-Output-loop-item-subvar-aaa
value: "{{item.aaa}}" value: "{{item.aaa}}"
dependencies: dependencies:
- produce-list-of-dicts - produce-list-of-dicts
name: for-loop-for-loop-00000003-3 name: for-loop-for-loop-00000003-3
template: for-loop-for-loop-00000003-3 template: for-loop-for-loop-00000003-3
withParam: "{{tasks.produce-list-of-dicts.outputs.parameters.produce-list-of-dicts-output}}" withParam: "{{tasks.produce-list-of-dicts.outputs.parameters.produce-list-of-dicts-Output}}"
- -
name: produce-list-of-dicts name: produce-list-of-dicts
template: produce-list-of-dicts template: produce-list-of-dicts
@ -525,11 +525,11 @@ spec:
outputs: outputs:
artifacts: artifacts:
- -
name: produce-list-of-dicts-output name: produce-list-of-dicts-Output
path: /tmp/outputs/Output/data path: /tmp/outputs/Output/data
parameters: parameters:
- -
name: produce-list-of-dicts-output name: produce-list-of-dicts-Output
valueFrom: valueFrom:
path: /tmp/outputs/Output/data path: /tmp/outputs/Output/data
- -
@ -589,11 +589,11 @@ spec:
outputs: outputs:
artifacts: artifacts:
- -
name: produce-list-of-ints-output name: produce-list-of-ints-Output
path: /tmp/outputs/Output/data path: /tmp/outputs/Output/data
parameters: parameters:
- -
name: produce-list-of-ints-output name: produce-list-of-ints-Output
valueFrom: valueFrom:
path: /tmp/outputs/Output/data path: /tmp/outputs/Output/data
- -
@ -653,11 +653,11 @@ spec:
outputs: outputs:
artifacts: artifacts:
- -
name: produce-list-of-strings-output name: produce-list-of-strings-Output
path: /tmp/outputs/Output/data path: /tmp/outputs/Output/data
parameters: parameters:
- -
name: produce-list-of-strings-output name: produce-list-of-strings-Output
valueFrom: valueFrom:
path: /tmp/outputs/Output/data path: /tmp/outputs/Output/data
- -
@ -711,10 +711,10 @@ spec:
outputs: outputs:
artifacts: artifacts:
- -
name: produce-str-output name: produce-str-Output
path: /tmp/outputs/Output/data path: /tmp/outputs/Output/data
parameters: parameters:
- -
name: produce-str-output name: produce-str-Output
valueFrom: valueFrom:
path: /tmp/outputs/Output/data path: /tmp/outputs/Output/data

18
sdk/python/tests/dsl/component_bridge_tests.py
View File

@ -194,3 +194,21 @@ class TestComponentBridge(unittest.TestCase):
full_command_line = task.command + task.arguments full_command_line = task.command + task.arguments
for arg in full_command_line: for arg in full_command_line:
self.assertNotIn('PipelineParam', arg) self.assertNotIn('PipelineParam', arg)
def test_converted_outputs(self):
component_text = textwrap.dedent('''\
outputs:
- name: Output 1
implementation:
container:
image: busybox
command:
- producer
- {outputPath: Output 1} # Outputs must be used in the implementation
'''
)
task_factory1 = load_component_from_text(component_text)
task1 = task_factory1()
self.assertSetEqual(set(task1.outputs.keys()), {'Output 1', 'output_1'})
self.assertIsNotNone(task1.output)