-
Notifications
You must be signed in to change notification settings - Fork 14
changes required to make subgraphs work in nvt #343
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
Conversation
Documentation preview |
@@ -512,7 +512,7 @@ def fit_phase(self, dataset, nodes, strict=False): | |||
|
|||
dask_client = global_dask_client() | |||
if dask_client: | |||
results = [r.result() for r in dask_client.compute(stats)] | |||
results = [r.result() for r in dask_client.compute(stats) if r is not None] |
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.
what's the scenario where the fit method of a on (Subraph op) returns None?
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.
Always, because it does not have stats itself. It has operators within it that have stats. So it will return the stats when its graph runs the fit (subgraph runs its own fit) but it wont bubble that up. So in the case where you only have subgraphs and no other stat operator on the main graph you end up in a situation where stats is empty and then there is not r
.
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.
stats is empty and then there is not r
if stats is empty ([]
) then the dask client compute also returns an empty list dask_client.compute([]) => []
. and r is never reached as None in that case.
root_schema, parents_schema, deps_schema, selector | ||
) | ||
self.graph = self.graph.construct_schema(input_schema) | ||
self.graph = self.graph.construct_schema(root_schema) |
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.
can you explain why the compute_input_schema method didn't work here?
This PR has some changes that fix some issues with graphs and it refactors the iter_nodes method in node to handle flattening of nodes in graphs that are subgraphs. These changes are required to make subgraphs work in nvtabular.