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.

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

fix: nested namespace holding kk mkl implementation #2100

Closed
wants to merge 1 commit into from
Closed

fix: nested namespace holding kk mkl implementation #2100

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion sparse/src/KokkosSparse_Utils_mkl.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ inline void mkl_internal_safe_call(sparse_status_t mkl_status, const char *name,
}

#define KOKKOSKERNELS_MKL_SAFE_CALL(call) \
KokkosSparse::Impl::mkl_internal_safe_call(call, #call, __FILE__, __LINE__)
::KokkosSparse::Impl::mkl_internal_safe_call(call, #call, __FILE__, __LINE__)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
::KokkosSparse::Impl::mkl_internal_safe_call(call, #call, __FILE__, __LINE__)
mkl_internal_safe_call(call, #call, __FILE__, __LINE__)

Copy link
Author

Choose a reason for hiding this comment

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

I'm not sure. I agree that the macro is defined here in a piece of code inside the KokkosSparse::Impl namespace. But, then, when the macro is used somewhere else, this namespace is not taken with it.

E.g. when it's used here

it's called in a piece of code in the KokkosSparse namespace without the Impl. So shouldn't we keep the proposed change?

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point this would obviously create issues. Would you mind moving the definition of the macro KOKKOSKERNELS_MKL_SAFE_CALL to line 232, just after the namespaces are closed?
My suspicion is at that point the leading :: in your fix might not be needed anymore?


inline sparse_operation_t mode_kk_to_mkl(char mode_kk) {
switch (toupper(mode_kk)) {
Expand Down
2 changes: 1 addition & 1 deletion sparse/tpls/KokkosSparse_spmv_bsrmatrix_tpl_spec_decl.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ namespace Impl {
#if (__INTEL_MKL__ > 2017)
// MKL 2018 and above: use new interface: sparse_matrix_t and mkl_sparse_?_mv()

using KokkosSparse::Impl::mode_kk_to_mkl;
using ::KokkosSparse::Impl::mode_kk_to_mkl;

inline matrix_descr getDescription() {
matrix_descr A_descr;
Expand Down