Netflix / metaflow

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

Bug: Runner API doesn't pass parameters to flows as expected #1885

Closed bmwilly closed 2 weeks ago

bmwilly commented 2 weeks ago

Calling a child flow from a parent flow using the Runner API is impossible if the two flows' parameter names are different. A reproducible example follows.

Given the flows child_flow.py:

import pandas as pd
from metaflow import FlowSpec, Parameter, step, current, card

class ChildFlow(FlowSpec):
    date_child_input = Parameter("date_child", type=str, required=True)
    x = Parameter("x", type=int, default=9)

    @step
    def start(self):
        self.date_child = pd.to_datetime(self.date_child_input)
        self.next(self.middle)

    @card
    @step
    def middle(self):
        self.y = 2 * self.x
        self.next(self.end)

    @step
    def end(self):
        print("done")

if __name__ == "__main__":
    ChildFlow()

and parent_flow.py:

from pathlib import Path
from metaflow import FlowSpec, Parameter, Runner, step, current

CWD = Path(__file__).parent

class ParentFlow(FlowSpec):
    date_input = Parameter("date", type=str, required=True)
    x = Parameter("x", type=int, default=9)

    @step
    def start(self):
        self.run_id = current.run_id
        self.next(self.child)

    @step
    def child(self):
        with Runner(str(CWD / "child_flow.py")).run(
            date_child=self.date_input,
            x=self.x,
        ) as running:
            self.child_run_id = running.run.id
        self.next(self.end)

    @step
    def end(self):
        print("done")

if __name__ == "__main__":
    ParentFlow()

running python parent_flow.py run --date 2024-01-01 leads to

  1. this error if we give date_child:
Runner(str(CWD / "child_flow.py")).run(date_child='2024-01-01')
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/Users/bwilliams/virtualenvs/default/3.10.14/lib/python3.10/site-packages/metaflow/runner/metaflow_runner.py", line 324, in run
    command = self.api(**self.top_level_kwargs).run(
  File "/Users/bwilliams/virtualenvs/default/3.10.14/lib/python3.10/site-packages/metaflow/runner/click_api.py", line 361, in _method
    method_params = _method_sanity_check(
  File "/Users/bwilliams/virtualenvs/default/3.10.14/lib/python3.10/site-packages/metaflow/runner/click_api.py", line 111, in _method_sanity_check
    raise ValueError("Missing argument: %s is required." % cli_name)
ValueError: Missing argument: date is required.
  1. this error if we give date:
Runner(str(CWD / "child_flow.py")).run(date='2024-01-01')
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/Users/bwilliams/virtualenvs/default/3.10.14/lib/python3.10/site-packages/metaflow/runner/metaflow_runner.py", line 324, in run
    command = self.api(**self.top_level_kwargs).run(
  File "/Users/bwilliams/virtualenvs/default/3.10.14/lib/python3.10/site-packages/metaflow/runner/click_api.py", line 361, in _method
    method_params = _method_sanity_check(
  File "/Users/bwilliams/virtualenvs/default/3.10.14/lib/python3.10/site-packages/metaflow/runner/click_api.py", line 111, in _method_sanity_check
    raise ValueError("Missing argument: %s is required." % cli_name)
ValueError: Missing argument: date_child is required.
  1. this error if we give both date and date_child:
Runner(str(CWD / "child_flow.py")).run(date='2024-01-01', date_child='2024-01-01')
Metaflow 2.12.3 executing ChildFlow for user:bwilliams
Usage: child_flow.py run [OPTIONS]
Try 'child_flow.py run --help' for help.

Error: no such option: --date  Did you mean --date_child?

(1) is the expected happy path; i.e., I don't think date should be a required argument.

sanowl commented 2 weeks ago

If the parameter names in the parent and child flows are different, the parameters do not map correctly, resulting in errors you can use the subprocess module to run the child flow and pass the parameters directly as command-line arguments.

bmwilly commented 2 weeks ago

@sanowl but is this expected behavior? Doesn't this defeat the purpose of the Runner API?

savingoyal commented 2 weeks ago

@bmwilly @sanowl #1886 should address this issue

bmwilly commented 2 weeks ago

@savingoyal thanks for the quick fix!