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

name variable confusion when using graph generators as tasks #207

Open
edan-bainglass opened this issue Aug 9, 2024 · 3 comments
Open

Comments

@edan-bainglass
Copy link
Member

edan-bainglass commented Aug 9, 2024

If one creates a WorkGraph with tasks that are calls to graph builders:

wg = WorkGraph("sweep")
for n in [1, 2]:
    wg.add_task(
        compute_graph(
            n, 
            suffix=f"for_{n}"
        )
    )

where compute_graph is

@task.graph_builder()
def compute_graph(n, suffix=""):
    suffix = f"_{suffix}" if suffix else ""
    wg = WorkGraph("compute{suffix}")
    ...
    return wg

the graph builder task will append a digit (starting at 1) to each compute task, such that the end result is

wg.tasks
NodeCollection(parent = "sweep", nodes = ["compute_for_11", "compute_for_22"])

where the first digit is part of the suffix, and the second is that appended by the graph builder.

There are use cases for which the auto-appending of the task counter is desired/required. @superstar54 please explain.

To avoid the above issue, @superstar54 recommends to explicitly name the tasks:

wg = WorkGraph("sweep")
for n in [1, 2]:
    wg.add_task(
        compute_graph(
            n, 
            suffix=f"for_{n}"
        ),
        name="compute_for_{n}",
    )

The suffix may still be useful if the compute_graph graph builder returns a workgraph that needs to assign the suffix to its own tasks. In this case, see issue #208.

@superstar54
Copy link
Member

Thanks for opening the issue! Take your workgraph as an example. Suppose your graph_builder generates a WorkGraph (wg1) with a name (e.g., compute1x1). this name will be used as the identifier, then you use this wg1 to create the task:

task1 = wg.add_task(wg1)
task2 = wg.add_task(wg1)

If there is no auto-appending of the task counter, both tasks will have the same name compute1x1, this will give an error. Thus, an auto-appending of the task counter is designed, and the name for task1 and task2 will be 'compute1x11', 'compute1x12'.

Of course, the user can provide the name explicitly, and in this case, the provided name will be used.

task1 = wg.add_task(wg1, name="compute1x1-abc")
task2 = wg.add_task(wg1, name="compute1x1-xyz")

@edan-bainglass
Copy link
Member Author

edan-bainglass commented Aug 10, 2024

All good. However, in my use case, the generated workgraph wg1 is named in the builder (see above). This ensures unique identifiers for the tasks, no?

But if I understand you correctly, the problem is not that wg1 is unnamed, but rather that the task is unnamed?

@superstar54
Copy link
Member

All good. However, in my use case, the generated workgraph wg1 is named in the builder (see above). This ensures unique identifiers for the tasks, no?

Yes, your wg1 is named, e.g., compute1. But what's if you using this wg1 to create multiple tasks?

wg = WorkGraph("sweep")
for n in [1, 2]:
    wg1 = compute_graph(n, suffix=f"for_{n}")
    wg.add_task(wg1)
    wg.add_task(wg1) 

This will give an error because of the name collision. Note, the WorkGraph only knows the name of the wg1; it doesn't aware the for loop, which you set the name for the wg1.

Using the same wg1 to create multiple tasks is useful in some cases. For example,

wg1 = compute_graph(1)
for i in range(n):
    task1 = wg.add_task(wg1, name="structure{i}")
    # Later you can update the input for the workgraph task
    task1.set({"define_structure.repetitions": [n, n, 1])

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants