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

fix(sdk): fix boolean default value serialization bug (#8444) 

* add test cases

* fix bug

* add v1 component yaml back compat changes

* update golden snapshot
This commit is contained in:
Connor McCarthy 2022-11-11 15:42:33 -08:00 committed by GitHub
parent 3f2e20d14f
commit 9e9e1081c6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 163 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

134
sdk/python/kfp/compiler/compiler_test.py
View File

@ -1356,5 +1356,139 @@ class TestMultipleExitHandlerCompilation(unittest.TestCase):
print_op(message='Inside second exit handler.')
class TestBoolInputParameterWithDefaultSerializesCorrectly(unittest.TestCase):
# test with default = True, may have false test successes due to protocol buffer boolean default of False
def test_python_component(self):
@dsl.component
def comp(boolean: bool = True) -> bool:
return boolean
# test inner component interface
self.assertEqual(
comp.pipeline_spec.components['comp-comp'].input_definitions
.parameters['boolean'].default_value.bool_value, True)
# test outer pipeline "wrapper" interface
self.assertEqual(
comp.pipeline_spec.root.input_definitions.parameters['boolean']
.default_value.bool_value, True)
def test_python_component_with_overrides(self):
@dsl.component
def comp(boolean: bool = False) -> bool:
return boolean
with tempfile.TemporaryDirectory() as tmpdir:
pipeline_spec_path = os.path.join(tmpdir, 'output.yaml')
compiler.Compiler().compile(
comp, pipeline_spec_path, pipeline_parameters={'boolean': True})
pipeline_spec = pipeline_spec_from_file(pipeline_spec_path)
# test outer pipeline "wrapper" interface
self.assertEqual(
pipeline_spec.root.input_definitions.parameters['boolean']
.default_value.bool_value, True)
def test_container_component(self):
@dsl.container_component
def comp(boolean: bool = True):
return dsl.ContainerSpec(image='alpine', command=['echo', boolean])
# test inner component interface
self.assertEqual(
comp.pipeline_spec.components['comp-comp'].input_definitions
.parameters['boolean'].default_value.bool_value, True)
# test pipeline "wrapper" interface
self.assertEqual(
comp.pipeline_spec.root.input_definitions.parameters['boolean']
.default_value.bool_value, True)
def test_container_component_with_overrides(self):
@dsl.container_component
def comp(boolean: bool = True):
return dsl.ContainerSpec(image='alpine', command=['echo', boolean])
with tempfile.TemporaryDirectory() as tmpdir:
pipeline_spec_path = os.path.join(tmpdir, 'output.yaml')
compiler.Compiler().compile(
comp, pipeline_spec_path, pipeline_parameters={'boolean': True})
pipeline_spec = pipeline_spec_from_file(pipeline_spec_path)
# test outer pipeline "wrapper" interface
self.assertEqual(
pipeline_spec.root.input_definitions.parameters['boolean']
.default_value.bool_value, True)
def test_pipeline_no_input(self):
@dsl.component
def comp(boolean: bool = True) -> bool:
return boolean
@dsl.pipeline
def pipeline_no_input():
comp()
# test inner component interface
self.assertEqual(
pipeline_no_input.pipeline_spec.components['comp-comp']
.input_definitions.parameters['boolean'].default_value.bool_value,
True)
def test_pipeline_with_input(self):
@dsl.component
def comp(boolean: bool = True) -> bool:
return boolean
@dsl.pipeline
def pipeline_with_input(boolean: bool = True):
comp(boolean=boolean)
# test inner component interface
self.assertEqual(
pipeline_with_input.pipeline_spec.components['comp-comp']
.input_definitions.parameters['boolean'].default_value.bool_value,
True)
# test pipeline interface
self.assertEqual(
pipeline_with_input.pipeline_spec.root.input_definitions
.parameters['boolean'].default_value.bool_value, True)
def test_pipeline_with_with_overrides(self):
@dsl.component
def comp(boolean: bool = True) -> bool:
return boolean
@dsl.pipeline
def pipeline_with_input(boolean: bool = False):
comp(boolean=boolean)
with tempfile.TemporaryDirectory() as tmpdir:
pipeline_spec_path = os.path.join(tmpdir, 'output.yaml')
compiler.Compiler().compile(
pipeline_with_input,
pipeline_spec_path,
pipeline_parameters={'boolean': True})
pipeline_spec = pipeline_spec_from_file(pipeline_spec_path)
# test inner component interface
self.assertEqual(
pipeline_spec.components['comp-comp'].input_definitions
.parameters['boolean'].default_value.bool_value, True)
# test pipeline interface
self.assertEqual(
pipeline_spec.root.input_definitions.parameters['boolean']
.default_value.bool_value, True)
if __name__ == '__main__':
unittest.main()

29
sdk/python/kfp/compiler/pipeline_spec_builder.py
View File

@ -360,9 +360,11 @@ def build_component_spec_for_task(
input_name].parameter_type = type_utils.get_parameter_type(
input_spec.type)
if input_spec.default is not None:
component_spec.input_definitions.parameters[
input_name].default_value.CopyFrom(
to_protobuf_value(input_spec.default))
_fill_in_component_input_default_value(
component_spec=component_spec,
input_name=input_name,
default_value=input_spec.default,
)
else:
component_spec.input_definitions.artifacts[
@ -404,9 +406,11 @@ def _build_component_spec_from_component_spec_structure(
input_name].parameter_type = type_utils.get_parameter_type(
input_spec.type)
if input_spec.default is not None:
component_spec.input_definitions.parameters[
input_name].default_value.CopyFrom(
to_protobuf_value(input_spec.default))
_fill_in_component_input_default_value(
component_spec=component_spec,
input_name=input_name,
default_value=input_spec.default,
)
else:
component_spec.input_definitions.artifacts[
@ -573,11 +577,15 @@ def _fill_in_component_input_default_value(
parameter_type = component_spec.input_definitions.parameters[
input_name].parameter_type
if pipeline_spec_pb2.ParameterType.NUMBER_INTEGER == parameter_type:
# cast to int to support v1 component YAML where NUMBER_INTEGER defaults are included as strings
# for example, input Limit: https://raw.githubusercontent.com/kubeflow/pipelines/60a2612541ec08c6a85c237d2ec7525b12543a43/components/datasets/Chicago_Taxi_Trips/component.yaml
component_spec.input_definitions.parameters[
input_name].default_value.number_value = default_value
input_name].default_value.number_value = int(default_value)
# cast to int to support v1 component YAML where NUMBER_DOUBLE defaults are included as strings
# for example, input learning_rate: https://raw.githubusercontent.com/kubeflow/pipelines/567c04c51ff00a1ee525b3458425b17adbe3df61/components/XGBoost/Train/component.yaml
elif pipeline_spec_pb2.ParameterType.NUMBER_DOUBLE == parameter_type:
component_spec.input_definitions.parameters[
input_name].default_value.number_value = default_value
input_name].default_value.number_value = float(default_value)
elif pipeline_spec_pb2.ParameterType.STRING == parameter_type:
component_spec.input_definitions.parameters[
input_name].default_value.string_value = default_value
@ -1061,9 +1069,8 @@ def modify_pipeline_spec_with_override(
# that match the pipeline inputs definition.
for input_name, input_value in (pipeline_parameters or {}).items():
if input_name in pipeline_spec.root.input_definitions.parameters:
pipeline_spec.root.input_definitions.parameters[
input_name].default_value.CopyFrom(
to_protobuf_value(input_value))
_fill_in_component_input_default_value(pipeline_spec.root,
input_name, input_value)
elif input_name in pipeline_spec.root.input_definitions.artifacts:
raise NotImplementedError(
'Default value for artifact input is not supported.')

22
sdk/python/test_data/pipelines/xgboost_sample_pipeline.yaml
View File

@ -7,7 +7,7 @@ components:
defaultValue: csv
parameterType: STRING
limit:
defaultValue: '1000'
defaultValue: 1000.0
parameterType: NUMBER_INTEGER
select:
defaultValue: trip_id,taxi_id,trip_start_timestamp,trip_end_timestamp,trip_seconds,trip_miles,pickup_census_tract,dropoff_census_tract,pickup_community_area,dropoff_community_area,fare,tips,tolls,extras,trip_total,payment_type,company,pickup_centroid_latitude,pickup_centroid_longitude,pickup_centroid_location,dropoff_centroid_latitude,dropoff_centroid_longitude,dropoff_centroid_location
@ -132,19 +132,19 @@ components:
defaultValue: gbtree
parameterType: STRING
label_column:
defaultValue: '0'
defaultValue: 0.0
parameterType: NUMBER_INTEGER
learning_rate:
defaultValue: '0.3'
defaultValue: 0.3
parameterType: NUMBER_DOUBLE
max_depth:
defaultValue: '6'
defaultValue: 6.0
parameterType: NUMBER_INTEGER
min_split_loss:
defaultValue: '0'
defaultValue: 0.0
parameterType: NUMBER_DOUBLE
num_iterations:
defaultValue: '10'
defaultValue: 10.0
parameterType: NUMBER_INTEGER
objective:
defaultValue: reg:squarederror
@ -174,16 +174,16 @@ components:
label_column_name:
parameterType: STRING
learning_rate:
defaultValue: '0.3'
defaultValue: 0.3
parameterType: NUMBER_DOUBLE
max_depth:
defaultValue: '6'
defaultValue: 6.0
parameterType: NUMBER_INTEGER
min_split_loss:
defaultValue: '0'
defaultValue: 0.0
parameterType: NUMBER_DOUBLE
num_iterations:
defaultValue: '10'
defaultValue: 10.0
parameterType: NUMBER_INTEGER
objective:
defaultValue: reg:squarederror
@ -885,4 +885,4 @@ root:
taskInfo:
name: xgboost-train-2
schemaVersion: 2.1.0
sdkVersion: kfp-2.0.0-beta.4
sdkVersion: kfp-2.0.0-beta.6