Skip to content

Fix openmp windows #41

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
t0mdavid-m merged 3 commits into from
Aug 17, 2025
Merged

Fix openmp windows #41

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
488c821
enable openmp
t0mdavid-m Aug 17, 2025
f882a28
bump version
t0mdavid-m Aug 17, 2025
499fdbe
enable openmp in docker
t0mdavid-m Aug 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .github/workflows/build-windows-executable-app.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ env:
OPENMS_VERSION: 3.2.0
PYTHON_VERSION: 3.11.0
# Name of the installer
APP_NAME: FLASHApp-0.9.2
APP_NAME: FLASHApp-0.9.4
APP_UpgradeCode: "69ae44ad-d554-4e3c-8715-7c4daf60f8bb"

jobs:
Expand Down Expand Up @@ -174,10 +174,10 @@ jobs:
WITH_GUI: "OFF"
ADDRESS_SANITIZER: "Off"
BUILD_TYPE: "Release"
OPENMP: "Off"
OPENMP: "On"
USE_STATIC_BOOST: "On"
# BUILD_FLAGS: "-p:CL_MPCount=2" # For VS Generator and MSBuild
BUILD_FLAGS: "-j2" # Ninja will otherwise use all cores (doesn't go well in GHA)
BUILD_FLAGS: "-j4" # Ninja will otherwise use all cores (doesn't go well in GHA)
CMAKE_CCACHE_EXE: "ccache"
CCACHE_BASEDIR: ${{ github.workspace }}
CCACHE_DIR: ${{ github.workspace }}/.ccache
Expand Down
2 changes: 1 addition & 1 deletion Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ RUN mkdir /openms-build
WORKDIR /openms-build

# Configure.
RUN /bin/bash -c "cmake -DCMAKE_BUILD_TYPE='Release' -DCMAKE_PREFIX_PATH='/OpenMS/contrib-build/;/usr/;/usr/local' -DHAS_XSERVER=OFF -DBOOST_USE_STATIC=OFF -DPYOPENMS=ON ../OpenMS -DPY_MEMLEAK_DISABLE=On"
RUN /bin/bash -c "cmake -DCMAKE_BUILD_TYPE='Release' -DCMAKE_PREFIX_PATH='/OpenMS/contrib-build/;/usr/;/usr/local' -DHAS_XSERVER=OFF -DBOOST_USE_STATIC=OFF -DPYOPENMS=ON ../OpenMS -DPY_MEMLEAK_DISABLE=On DOPENMP=ON"
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 17, 2025

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Fix missing -D prefix for OPENMP and place the source dir last in cmake invocation

The token DOPENMP=ON is missing the leading -, so CMake won’t treat it as a cache definition. Also, placing ../OpenMS before the final -D options is non-standard and brittle. Move all -D options before the source path and use -DOPENMP=ON. Consider consistent boolean casing as a minor cleanup.

[suggest_optional_refactor]

Apply this diff:

-RUN /bin/bash -c "cmake -DCMAKE_BUILD_TYPE='Release' -DCMAKE_PREFIX_PATH='/OpenMS/contrib-build/;/usr/;/usr/local' -DHAS_XSERVER=OFF -DBOOST_USE_STATIC=OFF -DPYOPENMS=ON ../OpenMS -DPY_MEMLEAK_DISABLE=On DOPENMP=ON"
+RUN /bin/bash -c "cmake -DCMAKE_BUILD_TYPE='Release' -DCMAKE_PREFIX_PATH='/OpenMS/contrib-build/;/usr/;/usr/local' -DHAS_XSERVER=OFF -DBOOST_USE_STATIC=OFF -DPYOPENMS=ON -DPY_MEMLEAK_DISABLE=ON -DOPENMP=ON ../OpenMS"

If OpenMS expects a different CMake toggle for OpenMP (e.g., -DOPENMS_OPENMP=ON or -DUSE_OPENMP=ON), adjust accordingly. To confirm the correct flag in the upstream OpenMS CMake, please run this quick web query:

🌐 Web query:

What is the CMake option used by the OpenMS project to enable OpenMP?

💡 Result:

The OpenMS project uses the OPENMS_USE_OPENMP CMake option to enable or disable OpenMP support during the build process. You can set this option by passing -DOPENMS_USE_OPENMP=ON to your cmake command when configuring the build.

Example usage:

cmake -DOPENMS_USE_OPENMP=ON ..

This option is specific to the OpenMS project and controls whether OpenMP parallelization is enabled in the compiled binaries. This approach is project-specific and complements the general CMake method for enabling OpenMP using the FindOpenMP module or manual flags for other projects[7][4].

If you are compiling OpenMS, be sure to use the exact name OPENMS_USE_OPENMP, as documented and used in the OpenMS source and CMake scripts.

Citations:

Correct OpenMP CMake flag and reorder options in Dockerfile

The Dockerfile’s CMake invocation needs the proper OpenMS-specific flag (OPENMS_USE_OPENMP) and all -D options placed before the source directory. Also normalize boolean values to uppercase.

• File: Dockerfile
• Line: 106

Suggested diff:

-RUN /bin/bash -c "cmake -DCMAKE_BUILD_TYPE='Release' -DCMAKE_PREFIX_PATH='/OpenMS/contrib-build/;/usr/;/usr/local' -DHAS_XSERVER=OFF -DBOOST_USE_STATIC=OFF -DPYOPENMS=ON ../OpenMS -DPY_MEMLEAK_DISABLE=On DOPENMP=ON"
+RUN /bin/bash -c "cmake \
+    -DCMAKE_BUILD_TYPE=Release \
+    -DCMAKE_PREFIX_PATH='/OpenMS/contrib-build/;/usr/;/usr/local' \
+    -DHAS_XSERVER=OFF \
+    -DBOOST_USE_STATIC=OFF \
+    -DPYOPENMS=ON \
+    -DPY_MEMLEAK_DISABLE=ON \
+    -DOPENMS_USE_OPENMP=ON \
+    ../OpenMS"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
RUN /bin/bash -c "cmake -DCMAKE_BUILD_TYPE='Release' -DCMAKE_PREFIX_PATH='/OpenMS/contrib-build/;/usr/;/usr/local' -DHAS_XSERVER=OFF -DBOOST_USE_STATIC=OFF -DPYOPENMS=ON ../OpenMS -DPY_MEMLEAK_DISABLE=On DOPENMP=ON"
RUN /bin/bash -c "cmake \
-DCMAKE_BUILD_TYPE=Release \
-DCMAKE_PREFIX_PATH='/OpenMS/contrib-build/;/usr/;/usr/local' \
-DHAS_XSERVER=OFF \
-DBOOST_USE_STATIC=OFF \
-DPYOPENMS=ON \
-DPY_MEMLEAK_DISABLE=ON \
-DOPENMS_USE_OPENMP=ON \
../OpenMS"
🤖 Prompt for AI Agents 
In Dockerfile around line 106, the CMake command uses an incorrect OpenMP flag
and places -D options after the source dir; update the RUN line so all -D
options come before the source directory, replace the incorrect DOPENMP flag
with -DOPENMS_USE_OPENMP=ON, and normalize boolean values to uppercase (ON/OFF)
for flags like PYOPENMS and PY_MEMLEAK_DISABLE; ensure CMAKE_PREFIX_PATH and
other -D settings remain but appear before the final "../OpenMS" argument.


# Build TOPP tools and clean up.
RUN make -j4 TOPP
Expand Down
2 changes: 1 addition & 1 deletion settings.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"app-name": "FLASHApp",
"github-user": "OpenMS",
"version": "0.9.2",
"version": "0.9.4",
"repository-name": "FLASHApp",
"analytics": {
"google-analytics": {
Expand Down
Loading