QA for #1377: do the original cuda_core/tests/ still work? #1394
+428
−282
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit migrates all code from cuda.core.experimental to cuda.core, completing the deprecation of the experimental namespace. Changes: - Move all files from cuda/core/experimental/ to cuda/core/ - Move include/ directory to _include/ (with leading underscore for implementation detail) - Update all imports from cuda.core.experimental.* to cuda.core.* - Update build_hooks.py to use cuda.core instead of cuda.core.experimental - Update pyproject.toml package-data paths - Update cuda/core/__init__.py to export all symbols - Update cuda/core/experimental/__init__.py for backward compatibility with deprecation warnings - Update Cython extern declarations to use _include/ instead of include/ - Fix import paths for _memory_pool (cuda.core._memory._memory_pool) - Update test files and test helpers to use new import paths All tests pass (1499 passed, 79 skipped).
Update all example files and test Cython files to use cuda.core instead of cuda.core.experimental: - Update all example imports from cuda.core.experimental.* to cuda.core.* - Update example utils imports from cuda.core.experimental.utils to cuda.core.utils - Update test Cython file imports - Update build_tests.sh include path from experimental/include to _include All example files now use the non-experimental import paths.
Update all documentation files to reference cuda.core instead of cuda.core.experimental: - api.rst: Change module from cuda.core.experimental to cuda.core - getting-started.rst: Update currentmodule and example imports - interoperability.rst: Update currentmodule - api_private.rst: Update currentmodule - conf.py: Update system import paths and excluded_dirs paths All documentation now reflects the new non-experimental API paths. Release notes files are left unchanged as they are historical.
Update GitHub issue templates and wheel merge script to reference cuda.core instead of cuda.core.experimental: - bug_report.yml: Update example references from cuda.core.experimental to cuda.core - feature_request.yml: Update example references from cuda.core.experimental to cuda.core - merge_cuda_core_wheels.py: Update to merge cuda/core/ instead of cuda/core/experimental/, and adopt logic that copies only binaries into versioned subdirectories while keeping Python modules in cuda/core/
…tal_deprecation_v1
This reverts commit e0f784d.
Fix the wheel merge script to copy files selectively instead of copying everything, which was causing import errors: - Copy binaries (.so, .pyd, .dll) to versioned directories (version-specific) - Copy Python files (.py) to versioned directories (needed for imports like utils.py) - Do NOT copy Cython files (.pyx, .pxd) to versioned directories The previous "copy everything" approach caused failures when testing CUDA 12.9.1 because CUDA 13 .pyx files (which reference version-specific C functions like cuMemGetMemPool) were being copied into cu12/ directories, causing import errors. This selective approach fixes both: - Original issue: utils.py import failures (now .py files are copied) - Current issue: cuMemGetMemPool errors (now .pyx files are NOT copied)
Add debugging output to show the directory structure of input and output wheels for troubleshooting. This will help diagnose issues with the wheel merge process. - Use Python's zipfile module (standard library) to list wheel contents - Add debugging output showing cuda/core/ directory structure for: * Each input wheel before merging * The output merged wheel after merging - Format output similar to unzip -l for readability - Filter output to show only cuda/core/ entries
Based on analysis of main branch's merge script and CI logs, updated the merge logic to: 1. Use shutil.copytree() to copy entire directory trees into versioned subdirectories (cu12/, cu13/), matching main's approach for experimental/ 2. Clean up main cuda/core/ directory to only keep: - __init__.py - _include/ directory - Versioned subdirectories (cu12/, cu13/) This ensures Python imports from versioned directories instead of main directory, which may contain binaries from a different CUDA version. 3. Add DRY helper function print_wheel_directory_structure() for debugging output, refactored from duplicate code. 4. Update docstring to reference cuda/core instead of experimental namespace. This fixes the ImportError issues where Python was loading CUDA 13 binaries when running on CUDA 12.9.1, because the main directory still contained .so files from the base wheel.
The merge script was removing _version.py and __init__.pxd from the main cuda/core/ directory, but these files are required: - _version.py: imported by __init__.py at module initialization - __init__.pxd: kept for Cython compatibility (matches main branch behavior) Updated the cleanup logic to preserve these files along with __init__.py, _include/, and versioned subdirectories (cu12/, cu13/). The code was also manually cleaned up for better readability and maintainability.
…touch() with os.truncate()
Migrated files from experimental namespace to core namespace: - _launch_config.pyx: Added new features from main, migrated imports - _launcher.pyx: Updated with main's changes, migrated imports - utils.py: Migrated from experimental.utils to core.utils - _kernel_arg_handler.pyx: Migrated from experimental to core - _kernel_arg_handler.pxd: Created in core (migrated from experimental) Updated test files to use core namespace: - test_memory.py: Migrated imports, preserved main's get_num_devices() changes - test_utils.py: Migrated imports, updated to use _layout module All conflicts resolved by migrating fresh from main's experimental namespace and applying namespace changes (experimental -> core).
- Add test_experimental_backward_compat.py from v0 branch - Remove __getattr__ from experimental/__init__.py per reviewer feedback (underscored modules are not public APIs) - Update test to expect AttributeError when accessing underscored modules - Document in module docstring that underscored modules are intentionally not accessible through experimental namespace - Move note about underscored modules to module docstring (per pre-commit)
Revert all changes under cuda_core/tests/ to match main branch, which uses cuda.core.experimental imports instead of cuda.core imports. This allows verification that the original tests still pass with the backward compatibility stubs, except possibly for direct imports of underscored (private) modules which are no longer accessible through the experimental namespace.
Contributor
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
Collaborator
Author
|
Output of |
Collaborator
Author
|
For comparison, the same output, but with the tests as under #1394 |
Collaborator
Author
|
I repeated the above after installing with Compact high-level comparison: The one extra test (1527 vs 1526) is Also: |
Collaborator
Author
|
I ran the equivalent build & tests commands against the |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
NOT FOR MERGING
The first commit (55db7f3) restores the original cuda_core/tests/
The second commit (c75fe3f) replaces all
cuda.core.experimental._withcuda.core._(because we intentionally don't provide backward compatibility stubs for private attributes).