cudf icon indicating copy to clipboard operation
cudf copied to clipboard

[DO NOT MERGE] Test pynvjitlink 0.2.0 compatibility

Open isVoid opened this issue 1 year ago • 9 comments

Using CI to test compatibility of pynvjitlink 0.2.0. Do not merge.

Description

Checklist

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

isVoid avatar Apr 16 '24 01:04 isVoid

Looks like we are having trouble picking up libkvikio packages in this CI job:

Could not solve for environment specs
The following package could not be installed
└─ libcudf-tests is not installable because it requires
   └─ libcudf 24.06.00a113 cuda12_240416_g768a984370_113, which requires
      └─ libkvikio 24.06.* , which does not exist (perhaps a missing channel).

However the libkvikio version 24.06 nightly packages do exist

jakirkham avatar Apr 16 '24 03:04 jakirkham

Looks like we are having trouble picking up libkvikio packages in this CI job:

Could not solve for environment specs
The following package could not be installed
└─ libcudf-tests is not installable because it requires
   └─ libcudf 24.06.00a113 cuda12_240416_g768a984370_113, which requires
      └─ libkvikio 24.06.* , which does not exist (perhaps a missing channel).

However the libkvikio version 24.06 nightly packages do exist

The x86 test seems correctly pulling libkvikio from rapidsai-nighly channel. https://github.com/rapidsai/cudf/actions/runs/8699225939/job/23857956270?pr=15540

isVoid avatar Apr 16 '24 04:04 isVoid

Looks like this may still be picking up the old pynvjitlink

jakirkham avatar Apr 16 '24 05:04 jakirkham

Looks like this may still be picking up the old pynvjitlink

Ah, the pynvjitlink lib is not marked as test dependency, but only as runtime dependency. In the test environment the pynvjitlink is pulled via a transitive dependency. I wonder if this is intended? @brandon-b-miller

isVoid avatar Apr 16 '24 05:04 isVoid

I believe we should have it in the testing environment too, yes. There's tests in cuDF that assume it's presence.

brandon-b-miller avatar Apr 16 '24 11:04 brandon-b-miller

I tried to add a specific dependency item for pynvjitlink but it seems like I made a mistake:

Traceback (most recent call last):
  File "/opt/conda/bin/rapids-dependency-file-generator", line 10, in <module>
    sys.exit(main())
  File "/opt/conda/lib/python3.10/site-packages/rapids_dependency_file_generator/cli.py", line 171, in main
    make_dependency_files(parsed_config, args.config, to_stdout)
  File "/opt/conda/lib/python3.10/site-packages/rapids_dependency_file_generator/rapids_dependency_file_generator.py", line 451, in make_dependency_files
    raise ValueError(
ValueError: No matching matrix found in 'test_python_cudf' for: {'cuda': '12.0', 'arch': 'aarch64', 'py': '3.10'}

isVoid avatar Apr 16 '24 15:04 isVoid

Seeing an interesting error in the CUDA 12 ARM Python 3.10 test job:

Traceback (most recent call last):
  File "/opt/conda/bin/rapids-dependency-file-generator", line 10, in <module>
    sys.exit(main())
  File "/opt/conda/lib/python3.10/site-packages/rapids_dependency_file_generator/cli.py", line 171, in main
    make_dependency_files(parsed_config, args.config, to_stdout)
  File "/opt/conda/lib/python3.10/site-packages/rapids_dependency_file_generator/rapids_dependency_file_generator.py", line 451, in make_dependency_files
    raise ValueError(
ValueError: No matching matrix found in 'test_python_cudf' for: {'cuda': '12.0', 'arch': 'aarch64', 'py': '3.10'}

Do we know why this is coming up?

jakirkham avatar Apr 16 '24 15:04 jakirkham

Commit 89bcda2c079a4a728d6f3a4abf4857c9d2a5de10 should fix the issues here.

Explanations:

There was an empty matrix like

- matrix:
  packages:

that was deleted in dependencies.yaml. We needed that empty matrix to provide a match for ARM (though it provides no packages).

There were also a bunch of unnecessary dependency entries in the test environment. These should already be provided as run dependencies of the pip/conda packages, so we shouldn't need to list them explicitly.

Finally, the conda recipe needed its pinning updated to pynvjitlink=0.2.0. This should test what you were hoping to test.

bdice avatar Apr 17 '24 01:04 bdice

Ah, the pynvjitlink lib is not marked as test dependency, but only as runtime dependency. In the test environment the pynvjitlink is pulled via a transitive dependency. I wonder if this is intended? @brandon-b-miller

This is expected behavior -- pynvjitlink is a runtime dependency (direct dependency of cudf, not a transitive dependency), and all runtime dependencies of cudf will be installed when you install cudf into the test environment. The section for "test dependencies" is only meant for packages that are not required by cudf but are needed to run its tests (e.g. pytest).

bdice avatar Apr 17 '24 01:04 bdice

I'm going to close this since we've been using newer versions of pynvjitlink safely for a while now.

vyasr avatar May 21 '24 17:05 vyasr