-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Enable or Disable cache for the Compiler #11209
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,7 @@ | |
|
||
import click | ||
from kfp import compiler | ||
from kfp.cli.utils import parsing | ||
from kfp.dsl import base_component | ||
from kfp.dsl import graph_component | ||
|
||
|
@@ -133,12 +134,19 @@ def parse_parameters(parameters: Optional[str]) -> Dict: | |
is_flag=True, | ||
default=False, | ||
help='Whether to disable type checking.') | ||
@click.option( | ||
'--enable-caching/--disable-caching', | ||
type=bool, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make sure to use |
||
default=None, | ||
help=parsing.get_param_descr(compiler.Compiler.compile, 'enable_caching'), | ||
) | ||
def compile_( | ||
py: str, | ||
output: str, | ||
function_name: Optional[str] = None, | ||
pipeline_parameters: Optional[str] = None, | ||
disable_type_check: bool = False, | ||
enable_caching: Optional[bool] = None, | ||
) -> None: | ||
"""Compiles a pipeline or component written in a .py file.""" | ||
pipeline_func = collect_pipeline_or_component_func( | ||
|
@@ -149,7 +157,8 @@ def compile_( | |
pipeline_func=pipeline_func, | ||
pipeline_parameters=parsed_parameters, | ||
package_path=package_path, | ||
type_check=not disable_type_check) | ||
type_check=not disable_type_check, | ||
enable_caching=enable_caching) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. once you update as per https://github.com/kubeflow/pipelines/pull/11209/files#r1759241709, |
||
|
||
click.echo(package_path) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,7 @@ | |
from kfp.compiler import pipeline_spec_builder as builder | ||
from kfp.dsl import base_component | ||
from kfp.dsl.types import type_utils | ||
from kfp.pipeline_spec import pipeline_spec_pb2 | ||
|
||
|
||
class Compiler: | ||
|
@@ -53,6 +54,7 @@ def compile( | |
pipeline_name: Optional[str] = None, | ||
pipeline_parameters: Optional[Dict[str, Any]] = None, | ||
type_check: bool = True, | ||
enable_caching: Optional[bool] = None, | ||
) -> None: | ||
"""Compiles the pipeline or component function into IR YAML. | ||
|
||
|
@@ -62,6 +64,12 @@ def compile( | |
pipeline_name: Name of the pipeline. | ||
pipeline_parameters: Map of parameter names to argument values. | ||
type_check: Whether to enable type checking of component interfaces during compilation. | ||
enable_caching: Whether or not to enable caching for the | ||
run. If not set, defaults to the compile time settings, which | ||
is ``True`` for all tasks by default, while users may specify | ||
different caching options for individual tasks. If set, the | ||
setting applies to all tasks in the pipeline (overrides the | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure if we want to override compile time settings, we just want to have an option to change the caching default. When we add the flag, this needs to happen for instance:
However, with your update of overriding in the client directly, task 3's caching might also get set to False. (Not entirely sure, something you would have to test and find out @diegolovison ) @gregsheremeta could elaborate |
||
compile time settings). | ||
""" | ||
|
||
with type_utils.TypeCheckManager(enable=type_check): | ||
|
@@ -78,9 +86,26 @@ def compile( | |
pipeline_parameters=pipeline_parameters, | ||
) | ||
|
||
if enable_caching is not None: | ||
override_caching_options(pipeline_spec, enable_caching) | ||
|
||
builder.write_pipeline_spec_to_file( | ||
pipeline_spec=pipeline_spec, | ||
pipeline_description=pipeline_func.description, | ||
platform_spec=pipeline_func.platform_spec, | ||
package_path=package_path, | ||
) | ||
|
||
|
||
def override_caching_options( | ||
pipeline_spec: pipeline_spec_pb2.PipelineSpec, | ||
enable_caching: bool, | ||
) -> None: | ||
"""Overrides caching options. | ||
|
||
Args: | ||
pipeline_spec: The PipelineSpec object to update in-place. | ||
enable_caching: Overrides options, one of True, False. | ||
""" | ||
for _, task_spec in pipeline_spec.root.dag.tasks.items(): | ||
task_spec.caching_options.enable_cache = enable_caching |
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.
Change this to
--disable-default-caching
to maintain precedence with other flags in this file. (like thetype_check
one)