search

Netflix / metaflow

:rocket: Build and manage real-life ML, AI, and data science projects with ease!
https://metaflow.org
Apache License 2.0
7.8k stars 738 forks source link

added local decorator #1824

Open vardhaman-surana opened 1 month ago

vardhaman-surana commented 1 month ago

for issue: https://github.com/Netflix/metaflow/issues/350

  1. added a new decorator: @local
  2. this will force steps to run locally even when doing --with batch and --with kubernetes, by preventing attachment of batch and kubernetes decorators to the steps which have @local attached to them already. to achieve this:
    • a classmethod should_attach is introduced in StepDecorator class which will always return True
    • that method is overridden in batch and kubernetes decorator classes to check for presence of local decorator on the step
    • the logic to check should_attach condition is implemented in _attach_decorators_to_step function inside decorators.py file
vardhaman-surana commented 1 month ago

@romain-intel @savingoyal I have updated the PR with the should attach class method approach. Please review once.

savingoyal commented 3 weeks ago

@vardhaman-surana, thanks for the PR. I would like to understand the use case better. Currently, we have @resources and @batch/@kubernetes decorators (and more coming). The attributes for @resources (CPU, mem, etc.) are a strict subset of @batch/@kubernetes today and are effectively ignored unless used --with batch/kubernetes. If the intention is that @local simply ignores the @batch/@kubernetes decorator - the step may not execute as expected if @batch/@kubernetes were providing a specific image or fiddling with tempfs settings, for example.

One possible workaround is to maybe have METAFLOW_DISABLE_DECOSPECS=kubernetes as an environment variable to disable all references of @kubernetes without providing an explicit @local decorator.

vardhaman-surana commented 3 weeks ago

@vardhaman-surana, thanks for the PR. I would like to understand the use case better. Currently, we have @resources and @batch/@kubernetes decorators (and more coming). The attributes for @resources (CPU, mem, etc.) are a strict subset of @batch/@kubernetes today and are effectively ignored unless used --with batch/kubernetes. If the intention is that @local simply ignores the @batch/@kubernetes decorator - the step may not execute as expected if @batch/@kubernetes were providing a specific image or fiddling with tempfs settings, for example.

One possible workaround is to maybe have METAFLOW_DISABLE_DECOSPECS=kubernetes as an environment variable to disable all references of @kubernetes without providing an explicit @Local decorator.

hi @savingoyal the use case as mentioned in the issue is

A possible use-case for this would be to annotate a GPU-heavy step that you want to run locally to save on AWS costs but take advantage of fast S3 access for other steps (from the EC2 machines batch uses).

i tried adding the local decorator based on this requirement mentioned in the issue. I am not sure if the env variable approach which you are suggesting will work for this case or not.