-
Notifications
You must be signed in to change notification settings - Fork 12
Feat: Make options dynamic #328
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.
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
Conversation
There was a problem hiding this 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 refactors build system options to make feature detection dynamic based on the environment instead of requiring explicit user configuration. The changes convert several hardcoded CMake options (HWLOC, HIP, and MPI) from manual opt-in flags to automatic detection that queries the system for available libraries.
Key Changes:
- Removed manual CMake options for HWLOC, HIP, and MPI support
- Implemented automatic
find_package()detection withQUIETflag for graceful fallback - Changed error handling from
FATAL_ERRORto informationalSTATUSmessages when optional dependencies are missing
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| cmake/modules/dftracer-utils.cmake | Added utility function to print all CMake variables for debugging |
| CMakeLists.txt | Converted HWLOC, HIP, and MPI from manual options to automatic detection; updated error handling to allow builds without optional dependencies |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
9939ac6 to
365f6dc
Compare
CMakeLists.txt
Outdated
| endif() | ||
|
|
||
| find_package(MPI COMPONENTS CXX QUIET) | ||
| if (MPI_CXX_FOUND AND DFTRACER_ENABLE_MPI) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a high level knob to enable detection based on user's request? The knob would be disabled by default
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DONE
365f6dc to
d75d2ce
Compare
|
Ok I have added a variable for automatic detection. That is off by default but if on, would ignore if user wanted to enable or disable feature. |
d75d2ce to
21854a1
Compare
|
@amarathe84 Can you check the PR |
cbba4c7 to
bb43f07
Compare
5643be7 to
4fb1cb9
Compare
4fb1cb9 to
a177da6
Compare
autobuild.sh
Outdated
| --enable-ftracing Enable function tracing | ||
| --enable-hip Enable HIP tracing | ||
| --enable-mpi Enable MPI support | ||
| --enable-dynamic Enable dynamic detection of MPI, HWLOC, and HIP at runtime |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor nit: should this be '--enable-dynamic-detection'? Existing option name suggests slightly different interpretation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
If users enables dynamic detection, dftracer will at runtime detect which modules (HIP, HWLOC, MPI) are available and load them accordingly.
a177da6 to
b5bf3e6
Compare
amarathe84
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good.
We make the following dynamic based on env.