feat: Add CPU features detection and stretched grid API (Phase 6)#19
Merged
feat: Add CPU features detection and stretched grid API (Phase 6)#19
Conversation
CPU Features API: - SIMD_NONE, SIMD_AVX2, SIMD_NEON constants - get_simd_arch() to detect SIMD architecture at runtime - get_simd_name() to get human-readable SIMD name - has_avx2(), has_neon(), has_simd() helper functions Grid Initialization: - create_grid_stretched() for hyperbolic cosine grid stretching - Clusters grid points toward domain center (symmetric)
There was a problem hiding this comment.
Pull request overview
This PR implements Phase 6 of the migration plan, adding CPU features detection and stretched grid initialization capabilities to the cfd_python package.
Key Changes:
- CPU Features API with SIMD architecture detection (AVX2, NEON, or none) and helper functions
- Stretched grid creation using hyperbolic cosine stretching that clusters points toward the domain center
- Comprehensive test suite with 26 tests covering all new functionality
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_cpu_features.py | Adds comprehensive test suite for CPU features detection and stretched grid creation with 26 test cases organized into 6 test classes |
| src/cfd_python.c | Implements C Python bindings for 5 CPU features functions and create_grid_stretched function with input validation and error handling |
| cfd_python/_loader.py | Updates extension loader to import and export 3 new SIMD constants and 6 new functions |
| cfd_python/init.py | Adds new CPU features API and grid initialization exports to all list |
| MIGRATION_PLAN.md | Marks Phase 6 as completed, documents actual implementation details and effort |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/cfd_python.c
Outdated
| // Grid Initialization Variants (Phase 6) | ||
| {"create_grid_stretched", create_grid_stretched_py, METH_VARARGS, | ||
| "Create a grid with stretched (non-uniform) spacing.\n\n" | ||
| "Uses hyperbolic cosine stretching to cluster points near boundaries.\n" |
There was a problem hiding this comment.
The documentation states the stretching "cluster points near boundaries" but this is incorrect. According to the PR description and the test comments, the hyperbolic cosine stretching actually clusters points toward the domain center, not the boundaries. The documentation should be updated to accurately reflect that this is a center-clustering stretching method.
… grid tests - Fix 17 reference leaks where PyList_SetItem failure didn't decref val - Skip TestCreateGridStretched tests due to C library formula bug - Document stretched grid bug (clusters center, not boundaries) - Update MIGRATION_PLAN.md and add ROADMAP.md for post-migration work
Add explicit NULL checks for PyLong_FromLong and PyUnicode_FromString return values in 8 functions to properly propagate memory errors: - get_last_status, get_last_error, get_error_string - bc_get_backend, bc_get_backend_name, backend_get_name - get_simd_arch, get_simd_name Add comprehensive tests in test_abi_compatibility.py covering: - Type verification for all fixed functions - Repeated call stress tests (100+ iterations) - Reference counting validation - Mixed function stress tests Also adds Phase 25 (ML/PINN) to ROADMAP.md.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Change coordinate keys from "x"/"y" to "x_coords"/"y_coords" to match the convention used by create_grid(). This makes the API consistent and predictable for users handling both grid types.
Update the documentation reference to use correct relative path (../cfd/ROADMAP.md) and clearer absolute reference (cfd-workspace/cfd/ROADMAP.md) for the stretched grid formula bug.
Update the docstring to accurately describe the current buggy behavior: - Clusters points toward CENTER (not boundaries) - Does not span full domain [xmin, xmax] - Both endpoints map to xmin Also update return value docs to use correct key names (x_coords, y_coords) and mark coordinate naming issue as fixed in MIGRATION_PLAN.md.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
CPU Features API:
Grid Initialization: