make setup.py check for AVX support

[poshul]

User description

Check before installing pyopenms that the processor has AVX support rather than throwing eldritch error messages at runtime.

Description

Address https://github.com/OpenMS/OpenMS/issues/7333 by failing at install time if the CPU doesn't support AVX. AVX has been standard for (physical) processors since the early 2010's so this is most likely to affect virtualization envs where changing the VM options solves the issue.

Checklist

• [x] Make sure that you are listed in the AUTHORS file
• [ ] Add relevant changes and new features to the CHANGELOG file
• [x] I have commented my code, particularly in hard-to-understand areas
• [ ] New and existing unit tests pass locally with my changes
• [x] Updated or added python bindings for changed or new classes (Tick if no updates were necessary.)

How can I get additional information on failed tests during CI

Click to expand

If your PR is failing you can check out

• The details of the action statuses at the end of the PR or the "Checks" tab.
• http://cdash.openms.de/index.php?project=OpenMS and look for your PR. Use the "Show filters" capability on the top right to search for your PR number. If you click in the column that lists the failed tests you will get detailed error messages.

Advanced commands (admins / reviewer only)

Click to expand

• /reformat (experimental) applies the clang-format style changes as additional commit. Note: your branch must have a different name (e.g., yourrepo:feature/XYZ) than the receiving branch (e.g., OpenMS:develop). Otherwise, reformat fails to push.
• setting the label "NoJenkins" will skip tests for this PR on jenkins (saves resources e.g., on edits that do not affect tests)
• commenting with rebuild jenkins will retrigger Jenkins-based CI builds

---

:warning: Note: Once you opened a PR try to minimize the number of pushes to it as every push will trigger CI (automated builds and test) and is rather heavy on our infrastructure (e.g., if several pushes per day are performed).

---

PR Type

enhancement, dependencies

---

Description

• Introduced a function check_avx_support to verify if the CPU supports the AVX instruction set, preventing runtime errors.
• Integrated the AVX support check into the setup process to fail early if the CPU does not support AVX.
• Added 'py-cpuinfo' as a required dependency to facilitate the AVX support check.

---

Changes walkthrough 📝

	Relevant files
Enhancement	setup.py Add AVX support check and update dependencies in setup.py src/pyOpenMS/setup.py Added a function to check for AVX support in the CPU. Integrated AVX support check into the setup process. Added 'py-cpuinfo' to the list of installation requirements. +19/-1

---

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

[github-actions[bot]]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Dependency Management The PR adds 'py-cpuinfo' as a dependency to check for AVX support. Ensure that this dependency is well-maintained and compatible with the project's existing environment to avoid potential conflicts or issues in deployment. Error Handling The function 'check_avx_support' exits the program using 'sys.exit(1)' upon failing to detect AVX support or if 'py-cpuinfo' is not installed. Consider raising an exception instead to allow higher-level handlers to manage the error more gracefully, especially in environments where 'setup.py' might be called as part of larger automated scripts.

[github-actions[bot]]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Possible bug	✅ Prevent potential crashes by adding error handling for missing 'flags' key in CPU info Suggestion Impact: The commit added error handling for the case where the 'flags' key might be missing in the CPU info dictionary, which aligns with the suggestion to prevent potential crashes. code diff: + if 'flags' not in info: + raise RuntimeError("ERROR: Cannot determine if AVX is supported. Something is wrong with get_cpu_info.\n") + elif 'avx' not in info['flags']: + raise RuntimeError("ERROR: The CPU (or virtualization environment) does not support AVX. AVX support is necessary for pyOpenMS.\n") Add error handling for potential KeyError when accessing 'flags' in the CPU info dictionary to prevent the script from crashing. src/pyOpenMS/setup.py [24] -if 'avx' not in info['flags']: +if 'flags' not in info or 'avx' not in info['flags']: Suggestion importance[1-10]: 9 Why: Adding error handling for a potential KeyError is crucial to prevent the script from crashing, making this a high-impact suggestion for ensuring robustness and reliability.	9
Enhancement	Enhance the CPU feature checks to include AVX2 alongside AVX Consider checking for AVX2 alongside AVX, as AVX2 is a more advanced and commonly used instruction set that might also be required. src/pyOpenMS/setup.py [24] -if 'avx' not in info['flags']: +if 'avx' not in info['flags'] or 'avx2' not in info['flags']: Suggestion importance[1-10]: 8 Why: Checking for AVX2 in addition to AVX is a significant enhancement, as AVX2 is a more advanced and commonly used instruction set, ensuring broader compatibility and performance.	8
Best practice	✅ Improve error handling by replacing sys.exit(1) with exceptions Suggestion Impact: The commit replaced instances of sys.exit(1) with raising RuntimeError exceptions, which aligns with the suggestion to improve error handling. code diff: - sys.exit(1) + if 'flags' not in info: + raise RuntimeError("ERROR: Cannot determine if AVX is supported. Something is wrong with get_cpu_info.\n") + elif 'avx' not in info['flags']: + raise RuntimeError("ERROR: The CPU (or virtualization environment) does not support AVX. AVX support is necessary for pyOpenMS.\n") except ImportError: - sys.stderr.write("ERROR: 'py-cpuinfo' is required to check AVX support.\n") - sys.exit(1) + raise RuntimeError("ERROR: 'py-cpuinfo' is required to check AVX support.\n") Replace the use of sys.exit(1) with raising an exception to allow for better error handling and integration with other Python code that might want to catch these errors. src/pyOpenMS/setup.py [26-29] -sys.exit(1) +raise RuntimeError("appropriate error message") Suggestion importance[1-10]: 7 Why: Replacing sys.exit(1) with exceptions improves error handling and allows for better integration with other Python code, making it a valuable enhancement for maintainability and robustness.	7
Maintainability	Improve code organization and clarity by importing cpuinfo at the top of the file Ensure that the cpuinfo module is imported at the beginning of the script to avoid repeated imports and clarify dependencies. src/pyOpenMS/setup.py [20] -from cpuinfo import get_cpu_info() +import cpuinfo Suggestion importance[1-10]: 5 Why: Importing cpuinfo at the top of the file enhances code organization and clarity, but the impact is moderate as it primarily affects readability and maintainability.	5

[timosachsenberg]

do we also need to check for avx2?

[poshul]

I don't think so. The minimum x86-64 microarchitecture level that supports avx also supports avx2 (x86-64-v3).

On Thursday, October 10, 2024, Timo Sachsenberg @.***> wrote:

do we also need to check for avx2?

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.< https://ci3.googleusercontent.com/meips/ADKq_NbJTNJM_LaSq_FHVVxc-_L7PAYU5TlKQvKYH0TXh6fXMsNRgqRZVQKZ_9rCBvzRx_V0bofPApBXIlg0ouBmtQWfAqSyYvp90CSm0DLlPnq0C5U9lI2NvKDR17o-752hzJ13eiIaZfYZZ3f3uhwWTsk5EL9HHcMwtA3FFi4qrOcHm585wh2Bav22_EwyTXtACrhs6HEq2Q8NOosk8hH9kVZmh3S7aV8zt2Z1P70KfkEqaq6g85_CVHg=s0-d-e1-ft#https://github.com/notifications/beacon/AABD3CZO63NJ2IL7CTYVNDLZ22BKXA5CNFSM6AAAAABPULAEMKWGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTUPLPSJW.gif>Message ID: @.***>

[jpfeuffer]

Good idea but let's try with a complete wheel on someone else's computer before merging.

[poshul]

Getting this to work consistantly across platforms looks like it's going to require moving the pyopenms building to a pyproject.toml which is out of scope for this specific issue I've opened an issue on this https://github.com/OpenMS/OpenMS/issues/7675 and am closing this PR