-
Notifications
You must be signed in to change notification settings - Fork 1.8k
docs(KEP): Support Literal pipeline input parameters #12400
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
docs(KEP): Support Literal pipeline input parameters #12400
Conversation
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
be4ee0b to
bb1bde8
Compare
c8e69dd to
9699275
Compare
0958925 to
612a840
Compare
| It is not currently possible to limit pipeline input to a specified set of values. Often pipeline authors want to restrict input in order to reduce unintended behavior and pipeline failure. This KEP proposes extending the valid pipeline input types to include Python Literal values, from the package `typing.Literal`. This change will involve updates to the SDK compiler, the pipeline run server and the UI input display. | ||
|
|
||
| ## Motivation | ||
| Valid pipeline input currently includes string, int, float, boolean, list and struct, as well as two custom types – TaskFinalStatus and TaskConfig. None of these choices allows a pipeline author to restrict input to one or more pre-determined options. When a pipeline run is executed with unintended inputs, the resulting failure unexpected behavior not only wastes time and resources but also creates a frustrating user experience. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically, the two custom types are for component inputs and not pipeline input parameters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
| It is not currently possible to limit pipeline input to a specified set of values. Often pipeline authors want to restrict input in order to reduce unintended behavior and pipeline failure. This KEP proposes extending the valid pipeline input types to include Python Literal values, from the package `typing.Literal`. This change will involve updates to the SDK compiler, the pipeline run server and the UI input display. | ||
|
|
||
| ## Motivation | ||
| Valid pipeline input currently includes string, int, float, boolean, list and struct, as well as two custom types – TaskFinalStatus and TaskConfig. None of these choices allows a pipeline author to restrict input to one or more pre-determined options. When a pipeline run is executed with unintended inputs, the resulting failure unexpected behavior not only wastes time and resources but also creates a frustrating user experience. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another motivation I'd add is to simplify the user experience. The user does not need to rely on the field being documented in the pipeline docstring to know what values are available. The KFP UI also doesn't currently surface these docstrings (the ODH/RHOAI dashboard does) if I recall correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
| #### Verify failure on the following: | ||
| | | SDK Compiler | Workflow Compiler | Execute Pipeline E2E | API Server: Verify Pipeline Run| | ||
| |-------------------------------------------------------------------------------|--------------|-------------------|----------------------|-----------------------| | ||
| | Pipeline & component-level Literal input:<br/> Literal elements do not match. | ✓ | X | X | X | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the SDK compiler should fail here as well to give early feedback.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Essentially, the behavior is that the pipeline input parameter must be a subset of the component input parameter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
| - Two examples of valid Literals that KFP would not compile: `Literal[“a”, 10, “b”]`; `Literal[False, “a”, 10]` | ||
| - Updates to the SDK compiler should be made primarily to the following files: `kfp/dsl/structures.py` and `kfp/dsl/types/type_utils.py` | ||
| - After the pipeline spec is populated, the SDK compiler should iterate through pipeline and component-level input and check that every element of a pipeline-level input Literal parameter is a valid input to the corresponding component Literal parameter. If not, compilation fails. | ||
| #### API Server |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you talk a bit more about the Driver behavior?
What comes to mind is that the output of a component could be input to another component's Literal input parameter. So Driver level validation is needed. It's implied in your test section but I'd like something more explicit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
| ### Design Details | ||
| In order to streamline the design and also to account for there being no `typing.Literal` counterpart in Go, a Literal parameter is represented similarly to a more typical parameter (e.g. int, string) with the only difference being an additional `literals` field: | ||
| #### InputSpec | ||
| `literals: Optional[List[Any]]` is added to the InputSpec class definition in `kfp/dsl/structures.py`: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can the type declaration be made tighter than List[Any]? In other words, can the Any be the types that are actually supported?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes - updated to Optional[List[str] | List[int] | List[float]]
|
|
||
| #### SDK Compiler | ||
| The following changes to the SDK compiler should be implemented first because the API server changes require a compiled pipeline YAML file. | ||
| - The current scope of this KEP extends to implementing Literal[string] and Literal[int] parameters |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Float is also mentioned in the test section.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
| } | ||
| ``` | ||
|
|
||
| #### SDK Compiler |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we also need to account for hardcoded values being passed to nested pipelines or components.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
Signed-off-by: agoins <[email protected]>
Signed-off-by: agoins <[email protected]>
87914c1 to
2da5215
Compare
Description of your changes:
KEP to implement #11385
Checklist: