Skip to content
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

TDQM hook with overrides #845

Open
zilto opened this issue Apr 25, 2024 · 2 comments
Open

TDQM hook with overrides #845

zilto opened this issue Apr 25, 2024 · 2 comments
Labels
bug Something isn't working

Comments

@zilto
Copy link
Collaborator

zilto commented Apr 25, 2024

Current behavior

When using hamilton.plugins.h_tdqm.ProgressBar(), the progress bar uses "the number of nodes to return" as denominator and "the number of nodes executed" as numerator. However, passing values to .execute(overrides=...) counts towrds the first, but not the latter. Code runs to completion but the progress bar completes without reaching 100%

Screenshots

image

Dataflow

# joke.py

def joke_prompt(topic: str) -> str:
    return f"Knock, knock. Who's there? {topic}"

def reply(joke_prompt: str) -> str:
    _, _, right = joke_prompt.partition("? ")
    return f"{right} who?"

def punchline(reply: str) -> str:
    left, _, _ = reply.partition(" ")
    return f"No, {left} MooOOooo"

Execution

import joke
from hamilton import driver
from hamilton.plugins.h_tqdm import ProgressBar
dr = (
    driver.Builder()
    .with_adapters(ProgressBar())
    .with_modules(joke)
    .build()
)

dr.execute(["joke_prompt", "reply"], overrides=dict(punchline="..."), inputs=topic("hello")).
@zilto zilto added triage label for issues that need to be triaged. bug Something isn't working good first issue Good for newcomers and removed triage label for issues that need to be triaged. labels Apr 25, 2024
@elijahbenizzy elijahbenizzy removed the good first issue Good for newcomers label Apr 25, 2024
@elijahbenizzy
Copy link
Collaborator

Hmm this might be a bit tricky -- we'll probably want to add a get_execution_plan() for the HamiltonGraph object. Otherwise we won't have access to the number. Otherwise we'll also want a overrides in the GraphExecutionHook (

class GraphExecutionHook(BasePreGraphExecute, BasePostGraphExecute):
).

Everything will be backwards compatible, but not super simple.

@zilto
Copy link
Collaborator Author

zilto commented Apr 26, 2024

This seems low priority, but good to have it documented! Thanks for the details

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants