Skip to content

Conversation

@shapiromatron
Copy link
Owner

No description provided.

@shapiromatron shapiromatron marked this pull request as ready for review August 12, 2025 02:30
Copilot AI review requested due to automatic review settings August 12, 2025 02:30
@shapiromatron shapiromatron merged commit b627e9f into main Aug 12, 2025
7 checks passed
@shapiromatron shapiromatron deleted the gsl branch August 12, 2025 02:30
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for two new scientific computing libraries: GSL (GNU Scientific Library) and NLopt (nonlinear optimization library). The changes integrate these libraries into the existing C++ Python extension module and provide Python bindings for basic functionality from each library.

  • Adds GSL integration with a Bessel function wrapper
  • Adds NLopt integration with a basic optimization function
  • Updates vcpkg triplet configurations to use static linking instead of dynamic

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/test_basic.py Adds test cases for the new GSL Bessel function and NLopt optimization functionality
src/cpp/main.cpp Implements C++ wrapper functions for GSL and NLopt and exposes them via pybind11
CMakeLists.txt Configures build system to find and link GSL and NLopt libraries from vcpkg
.github/workflows/wheels.yml Updates vcpkg triplet configurations from dynamic to static linking
.github/workflows/test.yml Updates vcpkg triplet configurations from dynamic to static linking

project(cppcore)

set(VCPKG_HOST_TRIPLET $ENV{VCPKG_HOST_TRIPLET})
set(VCPKG_HOST_TRIPLET $ENV{VCPKG_HOST_TRIPLET})
Copy link

Copilot AI Aug 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line appears to be a duplicate removal, but the same line is set again on line 5. This creates confusion about whether this is intentional duplication or an incomplete cleanup.

Suggested change
set(VCPKG_HOST_TRIPLET $ENV{VCPKG_HOST_TRIPLET})

Copilot uses AI. Check for mistakes.
pybind11_add_module(cppcore src/cpp/main.cpp)

include_directories(${CMAKE_SOURCE_DIR}/src/cpp ${EIGEN_PATH})
include_directories(${CMAKE_SOURCE_DIR}/src/cpp ${EIGEN_PATH} ${GSL_INCLUDE_DIR} ${NLOPT_DIR_INCLUDE_DIR})
Copy link

Copilot AI Aug 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using global 'include_directories' is discouraged in modern CMake. Consider using 'target_include_directories' exclusively, which you're already doing on line 31.

Suggested change
include_directories(${CMAKE_SOURCE_DIR}/src/cpp ${EIGEN_PATH} ${GSL_INCLUDE_DIR} ${NLOPT_DIR_INCLUDE_DIR})

Copilot uses AI. Check for mistakes.
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.

1 participant