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

Uses cudf 0.17 #494

Closed
wants to merge 2 commits into from
Closed

Uses cudf 0.17 #494

wants to merge 2 commits into from

Conversation

albert17
Copy link
Contributor

@albert17 albert17 commented Dec 14, 2020

Errors found when using cudf 0.17:

  1. KeyError: None in tests/unit/test_dask_nvt.py::test_dask_workflow_api_dlrm

    • This is due to a Dask bug. @rjzamora has already submitted a PR; and until we update to the next Dask version as a work around we can just avoid the flag index=False in NVTabular
  2. AssertionError: DataFrame.index are different in tests/unit/test_io.py::test_dask_dataset

    • This error is happening when reading multiple parquet files. Trying to fix it adding the flag check_index=False since we don't care about matching indices in NVTabular. That solved the Parquet error, but creates errors for csv. @rjzamora does not get the csv errors and it works for him.
  3. assert <NA> is None in test_ops.py::test_difference_lag

    • Having a frame with dtype: bool, we were doing the following operation mask[mask == False] = None. The resulting dataframe contains <NA> values instead of None. This is expected, cudf has stopped using None and instead is using <NA> to be coherent with pandas (cudf.NA).
  4. ValueError: Length mismatch: Expected axis has in test_workflow.py, test_torch_dataloader.py, test_ops.py, and test_io.py

    • We are getting many errors like ValueError: Length mismatch: Expected axis has 10 elements, new values have 2 elements. @rjzamora was guessing to be a cudf bug for reading BytesIO Objects, he has raisen an issue in cudf. Keith Kraus guess that it may be related to the boolean masking change if "doing something like gdf[gdf['col'] == x] where any value of gdf['col'] which is will return False for the purpose of boolean masking".

TODO: Fix errors:

  • Fix KeyError: None
  • Fix AssertionError: DataFrame.index
  • Fix assert <NA> is None
  • Fix ValueError: Length mismatch: Expected axis has

@albert17 albert17 linked an issue Dec 14, 2020 that may be closed by this pull request
@rjzamora
Copy link
Collaborator

@albert17 - thanks for compiling this!

Note that the workaround for (4) is to pass index=False when we crate new ParquetWriter objects. We shouldn't need to wait for a cudf fix for that bug, since NVTabular isn't relying on an index anywhere in the code base (for now).

@albert17
Copy link
Contributor Author

@rjzamora for (4) I have passed index=False when we crate new ParquetWriter objects. That fixed a good amount of errors, but still getting some errors related to test_join_external and test_join_external_workflow.

============================================================ short test summary info =============================================================
FAILED tests/unit/test_io.py::test_dask_dataset[2-parquet] - AssertionError: DataFrame.index are different
FAILED tests/unit/test_notebooks.py::test_criteo_notebook - subprocess.CalledProcessError: Command '['/opt/conda/envs/rapids/bin/python', '/tmp...
FAILED tests/unit/test_ops.py::test_join_external[True-left-host-cudf-parquet] - ValueError: Length mismatch: Expected axis has 189 elements, n...
FAILED tests/unit/test_ops.py::test_join_external[True-left-device-cudf-parquet] - ValueError: Length mismatch: Expected axis has 189 elements,...
FAILED tests/unit/test_ops.py::test_join_external[True-left-device-parquet-parquet] - ValueError: Length mismatch: Expected axis has 189 elemen...
FAILED tests/unit/test_ops.py::test_join_external[True-left-device-csv-parquet] - ValueError: Length mismatch: Expected axis has 189 elements, ...
FAILED tests/unit/test_ops.py::test_join_external[True-inner-host-cudf-parquet] - ValueError: Length mismatch: Expected axis has 189 elements, ...
FAILED tests/unit/test_ops.py::test_join_external[True-inner-device-cudf-parquet] - ValueError: Length mismatch: Expected axis has 189 elements...
FAILED tests/unit/test_ops.py::test_join_external[True-inner-device-parquet-parquet] - ValueError: Length mismatch: Expected axis has 189 eleme...
FAILED tests/unit/test_ops.py::test_join_external[True-inner-device-csv-parquet] - ValueError: Length mismatch: Expected axis has 189 elements,...
FAILED tests/unit/test_workflow.py::test_join_external_workflow[cat-parquet] - ValueError: Length mismatch: Expected axis has 189 elements, new...
FAILED tests/unit/test_workflow.py::test_join_external_workflow[cont-parquet] - ValueError: Length mismatch: Expected axis has 189 elements, ne...
FAILED tests/unit/test_workflow.py::test_join_external_workflow[feat-parquet] - ValueError: Length mismatch: Expected axis has 189 elements, ne...
FAILED tests/unit/test_workflow.py::test_join_external_workflow[all-parquet] - ValueError: Length mismatch: Expected axis has 189 elements, new...
====================================== 14 failed, 489 passed, 12 skipped, 36 warnings in 349.76s (0:05:49) =======================================

@benfred benfred closed this Jan 6, 2021
@benfred benfred deleted the branch NVIDIA-Merlin:new_api January 6, 2021 00:51
@albert17 albert17 mentioned this pull request Jan 7, 2021
6 tasks
@albert17 albert17 deleted the fix-cudf0.17 branch January 20, 2021 16:02
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

Successfully merging this pull request may close these issues.

3 participants