Include encode type in the error message when unsupported Parquet encoding is detected

[ZelboK]

Description

Per #14209 this will list out unsupported encodings that were found.

Checklist

• I am familiar with the Contributing Guidelines.
• New or existing tests cover these changes.
• The documentation is up to date with these changes.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[bdice]

@ZelboK Could you run pre-commit? See instructions in the Contributing Guide. I think this PR is going to fail C++ style checks from clang-format in its current state. https://github.com/rapidsai/cudf/blob/branch-23.12/CONTRIBUTING.md#using-pre-commit-hooks

[ZelboK]

@ZelboK Could you run pre-commit? See instructions in the Contributing Guide. I think this PR is going to fail C++ style checks from clang-format in its current state. https://github.com/rapidsai/cudf/blob/branch-23.12/CONTRIBUTING.md#using-pre-commit-hooks

Will do, thanks for mentioning!

[etseidl]

Thanks for this. Just a few suggestions :)

[wence-]

/ok to test

[ZelboK]

Question: I can't use BitAnd because the newly added kernel_error class's value() does not return a scoped enumeration. BitAnd uses SFINAE to only allow for scoped enums to use it.
I can:

1. Add new overload for BitAnd
2. Modify kernel_error to use scoped enumerations? (Might entail a lot more effort)
3. Maybe im forgetting some simple way to map the types properly. I'm still kinda new to C++ so i apologize.
4. Just not use BitAnd and do a regular bitwise operation.

I hesitate doing 1, or 2, because I don't have enough context for the codebase to know the friction it may cause. I imagine there's some reason BitAnd is constrained on scoped enums, and I don't have a good visual on how much work is required for 2.

[etseidl]

Looking good, thanks for the quick changes :) A few more minor suggestions.

[etseidl]

Sorry, a few more suggestions.

[GregoryKimball]

Thank you @ZelboK and @etseidl for iterating on this!

[ZelboK]

I was not getting the list of bad encodings. One problem was using the device iterators, but another issue is that count_page_headers will also receive the same error code, but the pages vector doesn't exist yet so we can't use that to get the set of bad encodings. I'd suggest testing for UNSUPPORTED_ENCODING in the error handling for count_page_headers() and not FAIL if that flag is set.

Is there any way I can test this behavior without having to rely on cudf python? Unfortunately I've run into a few blockers trying to get it set up. I'd like to finish this PR but it wouldn't be too efficient for me to offload the testing part to you like this. Builds are also quite slow without access to the cloud build artifacts.

I should be able to create unsupported encodings in a unit test I hope? I wasn't sure if the tests didn't have them because they weren't perceived to add value or if there was some complication I'm unaware of.

[etseidl]

Is there any way I can test this behavior without having to rely on cudf python?

You could write a small C++ prog that calls cudf::io::read_parquet(). Compiling it might be an adventure 😅

[ZelboK]

Is there any way I can test this behavior without having to rely on cudf python?

You could write a small C++ prog that calls cudf::io::read_parquet(). Compiling it might be an adventure 😅

Hm. Curious to know - when it comes to cuIO will I often need to run things from cudf python to test out functionality? If so that would justify me putting more effort into getting cudf python to work. I just have a lot more motivation for getting more experience with GPU programming than I do with tooling so I put it off lol. I'll spend a couple hours trying to figure this out

(btw this is the problem i get when trying to run pytest, forgive me, I don't program in python often 😅 ). Builds just fine, tests jut don't start up for me.

======================== test session starts ========================
platform linux -- Python 3.10.13, pytest-7.4.3, pluggy-1.3.0 -- /home/ksm/anaconda3/envs/cudf_dev/bin/python3.10
cachedir: .pytest_cache
benchmark: 4.0.0 (defaults: timer=time.perf_counter disable_gc=False min_rounds=5 min_time=0.000005 max_time=1.0 calibration_precision=10 warmup=False warmup_iterations=100000)
hypothesis profile 'default' -> database=DirectoryBasedExampleDatabase(PosixPath('/home/ksm/work/improving/cudf/python/cudf/.hypothesis/examples'))
rootdir: /home/ksm/work/improving/cudf/python/cudf
plugins: benchmark-4.0.0, cases-3.8.1, anyio-4.1.0, cov-4.1.0, hypothesis-6.92.0, xdist-3.5.0
collected 0 items / 1 error                                         

============================== ERRORS ===============================
___________________ ERROR collecting test session ___________________
/home/ksm/anaconda3/envs/cudf_dev/lib/python3.10/site-packages/_pytest/config/__init__.py:641: in _importconftest
    mod = import_path(conftestpath, mode=importmode, root=rootpath)
/home/ksm/anaconda3/envs/cudf_dev/lib/python3.10/site-packages/_pytest/pathlib.py:567: in import_path
    importlib.import_module(module_name)
/home/ksm/anaconda3/envs/cudf_dev/lib/python3.10/importlib/__init__.py:126: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
<frozen importlib._bootstrap>:1050: in _gcd_import
    ???
<frozen importlib._bootstrap>:1027: in _find_and_load
    ???
<frozen importlib._bootstrap>:1006: in _find_and_load_unlocked
    ???
<frozen importlib._bootstrap>:688: in _load_unlocked
    ???
/home/ksm/anaconda3/envs/cudf_dev/lib/python3.10/site-packages/_pytest/assertion/rewrite.py:186: in exec_module
    exec(co, module.__dict__)
benchmarks/conftest.py:59: in <module>
    from config import cudf  # noqa: W0611, E402, F401
benchmarks/common/config.py:46: in <module>
    import cudf  # noqa: W0611, F401
cudf/__init__.py:9: in <module>
    _setup_numba()
cudf/utils/_numba.py:122: in _setup_numba
    ptx_toolkit_version = _get_cuda_version_from_ptx_file(CC_60_PTX_FILE)
cudf/utils/_numba.py:157: in _get_cuda_version_from_ptx_file
    with open(path) as ptx_file:
E   FileNotFoundError: [Errno 2] No such file or directory: '/home/ksm/work/improving/cudf/python/cudf/cudf/utils/../core/udf/shim_60.ptx'
====================== short test summary info ======================
ERROR  - FileNotFoundError: [Errno 2] No such file or directory: '/home/k...
!!!!!!!!!!!!!! Interrupted: 1 error during collection !!!!!!!!!!!!!!!
========================= 1 error in 0.77s ==========================
(cudf_dev) ksm@Kashimo:~/work/improving/cudf/python/cudf$

Edit:

NVM! I fixed them :D

[DanialJavady96]

I was not getting the list of bad encodings. One problem was using the device iterators, but another issue is that count_page_headers will also receive the same error code, but the pages vector doesn't exist yet so we can't use that to get the set of bad encodings. I'd suggest testing for UNSUPPORTED_ENCODING in the error handling for count_page_headers() and not FAIL if that flag is set.

Might be misunderstanding, can you clarify a few things?
Just wondering, count_page_headers() without the pages vector will not be able to give anything past a general message no? Are you saying to relocate the logic in decode_page_headers over in count_page_headers()? I can use CUDF_LOG_ERROR to give a general message to the user.

Or are you saying that this logic

   if (error_code.value() != 0) {
    CUDF_FAIL("Parquet header parsing failed with code(s) " + error_code.str());
   }

is impacting the logic in decode_page_headers() because that shouldn't be the case.

Edit: Okay so it would seem that this is the case.

(cudf_dev) ksm@Kashimo:~/work/improving/cudf/python/cudf/cudf/tests$ pytest -s test_parquet.py::test_parquet_reader_unsupported_page_encoding
==================================== test session starts =====================================
platform linux -- Python 3.10.13, pytest-7.4.3, pluggy-1.3.0
benchmark: 4.0.0 (defaults: timer=time.perf_counter disable_gc=False min_rounds=5 min_time=0.000005 max_time=1.0 calibration_precision=10 warmup=False warmup_iterations=100000)
rootdir: /home/ksm/work/improving/cudf/python/cudf/cudf/tests
configfile: pytest.ini
plugins: benchmark-4.0.0, cases-3.8.1, anyio-4.1.0, cov-4.1.0, hypothesis-6.92.0, xdist-3.5.0
collected 1 item

test_parquet.py [ 51428][12:58:39:100371][error ] Parquet header parsing failed with code(s) 0x4
[ 51428][12:58:39:102664][error ] Unsupported encodings found: DELTA_LENGTH_BYTE_ARRAY

is what I get, if I change CUDF_FAIL to CUDF_LOG_ERROR instead, in count_page_headers(). It'll fail before decode_page_headers gets a chance to? Do we need both of these in both functions?

[vuule]

I think @etseidl 's idea is to completely ignore UNSUPPORTED_ENCODING error in count_page_headers so we can get to print the unsupported encoding types after DecodePageHeaders, where it will be reported again.

[etseidl]

One more small request 😅

Then I think it's good to go.

[vuule]

Thank you for the patch, looks good!
Few small suggestions

[ttnghia]

/ok to test

[etseidl]

All my nits have been picked 😄. LGTM

[vuule]

/merge