Skip to content

Conversation

@Grufoony
Copy link
Collaborator

@codecov
Copy link

codecov bot commented Dec 23, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.54%. Comparing base (5d36cb9) to head (c9e06b8).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #387   +/-   ##
=======================================
  Coverage   83.54%   83.54%           
=======================================
  Files          53       53           
  Lines        5401     5401           
  Branches      618      618           
=======================================
  Hits         4512     4512           
  Misses        878      878           
  Partials       11       11           
Flag Coverage Δ
unittests 83.54% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

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 addresses issue #386 by preventing the use of native architecture optimizations (-march=native) when building Python wheels for PyPI distribution. The change ensures that built wheels are compatible with a wider range of CPU architectures rather than being optimized only for the build machine's specific CPU.

Key changes:

  • Introduced a new CMake option DSF_OPTIMIZE_ARCH (default: ON) to control architecture-specific optimizations
  • Modified build flags to conditionally apply -march=native only when the option is enabled
  • Updated PyPI build workflows to explicitly disable architecture optimization via CMAKE_ARGS

Reviewed changes

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

File Description
src/dsf/dsf.hpp Bumped patch version from 3 to 4 to reflect the new release
CMakeLists.txt Added DSF_OPTIMIZE_ARCH option and made -march=native flag conditional in Release and Profile build modes
.github/workflows/pypi.yml Set CMAKE_ARGS: "-DDSF_OPTIMIZE_ARCH=OFF" for all three platform build jobs (Linux, macOS, Windows) to ensure portable wheels

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

CMakeLists.txt Outdated
if(CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Ofast -march=native -flto=auto")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Ofast -flto=auto")
if (DSF_OPTIMIZE_ARCH)
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

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

The spacing in the if statement is inconsistent with the rest of the file. The file consistently uses if( without a space between if and the opening parenthesis, but this line uses if ( with a space. Please change to if(DSF_OPTIMIZE_ARCH) to match the project's coding style.

Copilot uses AI. Check for mistakes.
CMakeLists.txt Outdated
if(CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Ofast -march=native -flto=auto -pg")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Ofast -flto=auto -pg")
if (DSF_OPTIMIZE_ARCH)
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

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

The spacing in the if statement is inconsistent with the rest of the file. The file consistently uses if( without a space between if and the opening parenthesis, but this line uses if ( with a space. Please change to if(DSF_OPTIMIZE_ARCH) to match the project's coding style.

Copilot uses AI. Check for mistakes.
@Grufoony Grufoony merged commit 5cd41d0 into main Dec 23, 2025
22 of 23 checks passed
@Grufoony Grufoony deleted the native branch December 23, 2025 15:38
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.

Avoid arch specific flags when compiling for pypi

2 participants