Skip to content
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

Warn instead of just ignoring user settings when kokkos-tools is disabled #5088

Merged
merged 4 commits into from
Jun 7, 2022

Conversation

dalg24
Copy link
Member

@dalg24 dalg24 commented Jun 5, 2022

Fix #5085
Raise a warning when the user passes a command line argument or sets an environment variable that is ignored because kokkos-tools is not actually enabled.

Drive-by change: refactor duplicated code to remove kokkos-tools flags

@dalg24
Copy link
Member Author

dalg24 commented Jun 5, 2022

This is only a draft because --kokkos-disable-warnings gets parsed after the tools command line arguments so the warnings are raised regardless :/

@dalg24 dalg24 force-pushed the warn_tools_cmd_line_when_disabled branch 2 times, most recently from 9d24b2c to 53304d3 Compare June 5, 2022 19:14
@dalg24
Copy link
Member Author

dalg24 commented Jun 6, 2022

This is only a draft because --kokkos-disable-warnings gets parsed after the tools command line arguments so the warnings are raised regardless :/

On second thought, I suggest we merge this and consider this as a known defect in 3.7 that we will fix in 4.0
When we fix #5087 we will be able to parse the Kokkos Core options first which will resolve that defect.

@dalg24 dalg24 marked this pull request as ready for review June 6, 2022 15:44
@dalg24
Copy link
Member Author

dalg24 commented Jun 6, 2022

Clang+CUDA build failure is clearly unrelated. This is ready for review.

core/src/impl/Kokkos_Profiling.cpp Outdated Show resolved Hide resolved
core/src/impl/Kokkos_Profiling.cpp Outdated Show resolved Hide resolved
@dalg24 dalg24 force-pushed the warn_tools_cmd_line_when_disabled branch from 53304d3 to d1a50a2 Compare June 6, 2022 16:14
Copy link
Contributor

@masterleinad masterleinad left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@dalg24
Copy link
Member Author

dalg24 commented Jun 7, 2022

Ignoring OPENMPTARGET-Clang failure (timeout in KokkosCore_UnitTest_Serial1)

@dalg24 dalg24 merged commit bd0b485 into kokkos:develop Jun 7, 2022
@dalg24 dalg24 deleted the warn_tools_cmd_line_when_disabled branch June 15, 2022 14:24
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.

None yet

3 participants