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

Temporarily disable the deletion of the dask dataframe #3814

Merged
merged 39 commits into from
Sep 28, 2023

Conversation

jnke2016
Copy link
Contributor

@jnke2016 jnke2016 commented Aug 22, 2023

temporarily disable the deletion of the dask dataframe

@jnke2016 jnke2016 requested a review from a team as a code owner August 22, 2023 03:21
@jnke2016 jnke2016 changed the title Alternatively copy the internal input dataframe Temporarily disable the deletion of the dask dataframe Aug 23, 2023
@jnke2016
Copy link
Contributor Author

/ok to test

@@ -337,7 +334,7 @@ def __from_edgelist(
)
for w, edata in ddf.items()
}
del ddf
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this deletion come after the computation below is done anyway? If you are building graph from ddf, then you cannot release the keys of ddf until delayed_tasks_d is finished computing/persisting.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure why the keys were deleted before delayed_tasks_d computes. My PR essentially removed that deletion and it worked. I am not sure if doing it after delayed_tasks_d computes will make a difference but let me try.

@rlratzel rlratzel removed the DO NOT MERGE Hold off on merging; see PR for details label Sep 27, 2023
@jnke2016
Copy link
Contributor Author

/ok to test

@jnke2016
Copy link
Contributor Author

/ok to test

@jnke2016
Copy link
Contributor Author

/ok to test

@rlratzel rlratzel added the DO NOT MERGE Hold off on merging; see PR for details label Sep 27, 2023
@VibhuJawa
Copy link
Member

Looks like this PR still failed:

opt/conda/envs/test/lib/python3.10/site-packages/cudf/core/column_accessor.py:512: KeyError
----------------------------- Captured stderr call -----------------------------
2023-09-27 19:30:22,427 - distributed.worker - WARNING - Compute Failed
Key:       ('getitem-45f935df74439fffe02ff7ccf637a452', 0)
Function:  subgraph_callable-51d0a705-adef-4733-adbc-41225d92
args:      (Empty DataFrame
Columns: []
Index: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84, 85, 86, 87, 88, 89, 90, 91, 92, 93, 94, 95, 96, 97, 98, 99, ...]

[151 rows x 0 columns], 'src')
kwargs:    {}
Exception: "KeyError('src')"
``

@jnke2016
Copy link
Contributor Author

/ok to test

@naimnv naimnv removed the DO NOT MERGE Hold off on merging; see PR for details label Sep 28, 2023
@jnke2016
Copy link
Contributor Author

/ok to test

@raydouglass raydouglass removed the request for review from a team September 28, 2023 16:13
@raydouglass
Copy link
Member

Removing ops-codeowners from the required reviews since it doesn't seem there are any file changes that we're responsible for. Feel free to add us back if necessary.

@rlratzel
Copy link
Contributor

/merge

@rapids-bot rapids-bot bot merged commit f57119b into rapidsai:branch-23.10 Sep 28, 2023
72 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants