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

Template usage in kernel_dispatch.hpp is difficult to interpret (for SWIG)? #4530

Closed
sshiraiwa opened this issue Oct 3, 2024 · 12 comments
Closed
Assignees

Comments

@sshiraiwa
Copy link
Member

I am opening this message here to get help from C++ experts.
With recent MFEM master, I am having a problem of running SWIG to generate PyMFEM.
It somehow crashes. Looking into the root cause, it appears that SWIG have a difficulty tin understanding how template is used in kernel_dispatch.hpp.
When I give this header file to SWIG separately, it spits out the following error message
/home/sshiraiw/venvs/env20220809/lib/python3.9/site-packages/mfem/external/ser/include/mfem/fem/kernel_dispatch.hpp:128: Error: Template partial specialization has more arguments than primary template 4 1.
Can someone tell what is going on? Is this an issue which has to be fixed in SWIG?

@pazner
Copy link
Member

pazner commented Oct 3, 2024

That part of the code uses some kind of complicated templates and macros. Is it possible to simply skip that file when running SWIG? It's not really user-facing, so we don't actually need to generate any Python bindings for this.

@sshiraiwa
Copy link
Member Author

I already tried to skip it by %ignore directive in SWIG, but it does not work, probably because this line does not make sense at all for SWIG. While we don't need to expose it in Python binding, SWIG need to know the MFEM_REGISTER_KERNELS macro, which is defined above in the same file, in order to interpret bilininteg.hpp and quadinterpolator.hpp.

I will try bit more to find a work-around.

@sshiraiwa
Copy link
Member Author

I think I was able to fix the original issue. Then, I am facing another build issue on MacOS.
The compiler is complaining the same header file. This is not Python wrapper build. It fails the step to build
MFEM itself. Do you have any suggestion?

In file included from /Users/syunichishiraiwa/venvs/env20230615/PyMFEM/external/mfem/fem/qinterp/eval_by_nodes.cpp:12:
In file included from /Users/syunichishiraiwa/venvs/env20230615/PyMFEM/external/mfem/fem/qinterp/../quadinterpolator.hpp:16:
/Users/syunichishiraiwa/venvs/env20230615/PyMFEM/external/mfem/fem/qinterp/../kernel_dispatch.hpp:104:23: error: implicit instantiation of undefined template 'std::hash<mfem::QVectorLayout>'
      auto lhs_hash = std::hash<THead>()(std::get<Index>(value));
                      ^
/Users/syunichishiraiwa/venvs/env20230615/PyMFEM/external/mfem/fem/qinterp/../kernel_dispatch.hpp:105:23: note: in instantiation of function template specialization 'mfem::KernelDispatchKeyHash<int, mfem::QVectorLayout, int, int, int>::operator()<5UL, mfem::QVectorLayout, int, int, int>' requested here
      auto rhs_hash = operator()<N, TTail...>(value);
                      ^
/Users/syunichishiraiwa/venvs/env20230615/PyMFEM/external/mfem/fem/qinterp/../kernel_dispatch.hpp:112:14: note: in instantiation of function template specialization 'mfem::KernelDispatchKeyHash<int, mfem::QVectorLayout, int, int, int>::operator()<5UL, int, mfem::QVectorLayout, int, int, int>' requested here
      return operator()<sizeof...(KernelParameters),KernelParameters...>(value);
             ^
/Library/Developer/CommandLineTools/SDKs/MacOSX12.sdk/usr/include/c++/v1/unordered_map:474:17: note: in instantiation of member function 'mfem::KernelDispatchKeyHash<int, mfem::QVectorLayout, int, int, int>::operator()' requested here
        {return static_cast<const _Hash&>(*this)(__x);}
                ^
/Library/Developer/CommandLineTools/SDKs/MacOSX12.sdk/usr/include/c++/v1/__hash_table:2072:21: note: in instantiation of member function 'std::__unordered_map_hasher<std::tuple<int, mfem::QVectorLayout, int, int, int>, std::__hash_value_type<std::tuple<int, mfem::QVectorLayout, int, int, int>, void (*)(int, const double *, const double *, double *, int, int, int)>, mfem::KernelDispatchKeyHash<int, mfem::QVectorLayout, int, int, int>, std::equal_to<std::tuple<int, mfem::QVectorLayout, int, int, int>>, true>::operator()' requested here
    size_t __hash = hash_function()(__k);
                    ^
/Library/Developer/CommandLineTools/SDKs/MacOSX12.sdk/usr/include/c++/v1/unordered_map:1743:21: note: in instantiation of function template specialization 'std::__hash_table<std::__hash_value_type<std::tuple<int, mfem::QVectorLayout, int, int, int>, void (*)(int, const double *, const double *, double *, int, int, int)>, std::__unordered_map_hasher<std::tuple<int, mfem::QVectorLayout, int, int, int>, std::__hash_value_type<std::tuple<int, mfem::QVectorLayout, int, int, int>, void (*)(int, const double *, const double *, double *, int, int, int)>, mfem::KernelDispatchKeyHash<int, mfem::QVectorLayout, int, int, int>, std::equal_to<std::tuple<int, mfem::QVectorLayout, int, int, int>>, true>, std::__unordered_map_equal<std::tuple<int, mfem::QVectorLayout, int, int, int>, std::__hash_value_type<std::tuple<int, mfem::QVectorLayout, int, int, int>, void (*)(int, const double *, const double *, double *, int, int, int)>, std::equal_to<std::tuple<int, mfem::QVectorLayout, int, int, int>>, mfem::KernelDispatchKeyHash<int, mfem::QVectorLayout, int, int, int>, true>, std::allocator<std::__hash_value_type<std::tuple<int, mfem::QVectorLayout, int, int, int>, void (*)(int, const double *, const double *, double *, int, int, int)>>>::__emplace_unique_key_args<std::tuple<int, mfem::QVectorLayout, int, int, int>, const std::piecewise_construct_t &, std::tuple<const std::tuple<int, mfem::QVectorLayout, int, int, int> &>, std::tuple<>>' requested here
    return __table_.__emplace_unique_key_args(__k,
                    ^
/Users/syunichishiraiwa/venvs/env20230615/PyMFEM/external/mfem/fem/qinterp/../kernel_dispatch.hpp:174:13: note: in instantiation of member function 'std::unordered_map<std::tuple<int, mfem::QVectorLayout, int, int, int>, void (*)(int, const double *, const double *, double *, int, int, int), mfem::KernelDispatchKeyHash<int, mfem::QVectorLayout, int, int, int>>::operator[]' requested here
            Kernels::Get().table[param_tuple] =
            ^
/Users/syunichishiraiwa/venvs/env20230615/PyMFEM/external/mfem/fem/qinterp/eval_by_nodes.cpp:27:63: note: in instantiation of member function 'mfem::KernelDispatchTable<mfem::QuadratureInterpolator::TensorEvalKernels, void (*)(int, const double *, const double *, double *, int, int, int), mfem::internal::KernelTypeList<int, mfem::QVectorLayout, int, int, int>, mfem::internal::KernelTypeList<int>>::Specialization<2, mfem::QVectorLayout::byNODES, 1, 3, 3>::Opt<1>::Add' requested here
   k::Specialization<2,QVectorLayout::byNODES,1,3,3>::Opt<1>::Add();
                                                              ^
/Library/Developer/CommandLineTools/SDKs/MacOSX12.sdk/usr/include/c++/v1/__memory/shared_ptr.h:1710:50: note: template is declared here
template <class _Tp> struct _LIBCPP_TEMPLATE_VIS hash;
                                                 ^

@pazner
Copy link
Member

pazner commented Oct 11, 2024

What compiler (and version) are you using?

It's related to this. We can add a fix/workaround if needed.

@sshiraiwa
Copy link
Member Author

@pazner, Thank you for prompt response. The compiler is from Apple default. (/usr/bin/c++), and --version spits out
the following

Apple clang version 14.0.0 (clang-1400.0.29.202)
Target: arm64-apple-darwin21.6.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin

Typical compiler command line generated by CMAKE is as follows

/usr/bin/c++ -DMFEM_CONFIG_FILE=\"/.../mfem/cmbuild_ser/config/_config.hpp\" -Dmfem_EXPORTS -I/.../mfem/cmbuild_ser -I/.../mfem -std=c++11 -O3 -DNDEBUG -std=c++11 -arch arm64 -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX12.sdk -fPIC -MD -MT CMakeFiles/mfem.dir/fem/fe/fe_nd.cpp.o -MF CMakeFiles/mfem.dir/fem/fe/fe_nd.cpp.o.d -o CMakeFiles/mfem.dir/fem/fe/fe_nd.cpp.o -c /.../mfem/fem/fe/fe_nd.cpp

Thank you in advance. The MacOS version is bit old (Monterey) due to some reason. I hope this is not the problem.

@pazner
Copy link
Member

pazner commented Oct 14, 2024

Actually I think the problem has to do with the version of the standard library rather than the compiler version:

Are you for some reason using an old version of the standard library? This was fixed in libstdc++ (gcc's standard library) in version 6.1.0 and libc++ (clang's standard library) in version 4 (both in 2013).

@sshiraiwa
Copy link
Member Author

@panzer, thank you for your advise. After a lot of trial and error, I found that I need to set -std=c++14, instead of c++11. Following is an example of compiler call, which works for me. Note that -std=c++14 is added before -std=c++11 is specified. The first one is enforced using -DCMAKE_CXX_FLAGS flag when calling CMAKE. There is still -std=c++11 remained, perhaps coming from MFEM's CMAKE default setting?
I am not sure what is the implication. Is MFEM using something from the c++14 standard not officially supported in C++11, and my compiler caught it ??? Anyhow, there is a workaround for this.

/usr/bin/c++ 
-DMFEM_CONFIG_FILE=\"./PyMFEM/external/mfem/cmbuild_ser/config/_config.hpp\"
-Dmfem_EXPORTS -I./PyMFEM/external/mfem/cmbuild_ser 
-./PyMFEM/external/mfem 
-std=c++14 O3 -DNDEBUG -std=c++11 -arch arm64 
-isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX12.sdk
-fPIC -MD -MT CMakeFiles/mfem.dir/fem/qinterp/grad_phys_by_nodes.cpp.o 
-MF CMakeFiles/mfem.dir/fem/qinterp/grad_phys_by_nodes.cpp.o.d 
-o CMakeFiles/mfem.dir/fem/qinterp/grad_phys_by_nodes.cpp.o 
-c ./PyMFEM/external/mfem/fem/qinterp/grad_phys_by_nodes.cpp

@pazner
Copy link
Member

pazner commented Oct 20, 2024

Hi @sshiraiwa,

The problem you were running into is caused by the compiler flag:

-isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX12.sdk

This is sets the "system root directory", which will cause the compiler to use the C++ standard library found at /Library/Developer/CommandLineTools/SDKs/MacOSX12.sdk/usr/include/c++/v1. This is an old version of the standard library. If you use instead -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX13.sdk (or later), I believe it should work.

Note that this is not exactly a C++14 vs. C++11 issue — it's not about the language per se, but rather about the implementation of the standard library. This should work with a C++11 compiler, as long as the standard library supports hashing enums.

@sshiraiwa
Copy link
Member Author

Hi @pazner
Okay. Thanks. The line of sysroot is set, because the Python I am using (system Python) is built with this version of SDK, and difficult to change (I will have a different compatibility problem), unless I switch to another Python built differently, like Brew.

I think we can close this since there is work around and the root cause of issue is identified.
Do you anticipate any issue of setting -std=C++14 as above? Does it make the code break?

@pazner
Copy link
Member

pazner commented Oct 21, 2024

There shouldn't be any issue setting -std=c++14 (or any standard newer than C++11), but just FYI that fix doesn't work for me.

Are you running on an older version of Mac OS?

BTW, this is the workaround I mentioned earlier. Does it also fix the issue for you? I'm hesitant to recommend merging this because the same code will need to be added for any enum used in the kernel dispatch and it's really just a workaround for pretty old versions of the standard library, once we move to C++17 (hopefully soon!) it should be irrelevant.

diff --git a/fem/fespace.hpp b/fem/fespace.hpp
index 7edb9e640..b8a006964 100644
--- a/fem/fespace.hpp
+++ b/fem/fespace.hpp
@@ -47,6 +47,10 @@ public:
    static void DofsToVDofs(int ndofs, int vdim, Array<int> &dofs);
 };
 
+// Note: std::hash is specialized for QVectorLayout below (outside the mfem
+// namespace). This is a workaround for a defect in older versions of the
+// standard library. Hashing is needed for kernel dispatch.
+
 /// @brief Type describing possible layouts for Q-vectors.
 /// @sa QuadratureInterpolator and FaceQuadratureInterpolator.
 enum class QVectorLayout
@@ -1385,6 +1389,15 @@ inline bool UsesTensorBasis(const FiniteElementSpace& fes)
 /// elements, otherwise, return NATIVE.
 ElementDofOrdering GetEVectorOrdering(const FiniteElementSpace& fes);
 
-}
+} // namespace mfem
+
+template <> struct std::hash<mfem::QVectorLayout>
+{
+   size_t operator()(const mfem::QVectorLayout &x) const
+   {
+      return std::hash<int>()(static_cast<int>(x));
+      // Could also just do: return static_cast<size_t>(x);
+   }
+};
 
 #endif

@sshiraiwa
Copy link
Member Author

Thank you. As I wrote earlier, the MacOS version is bit old (Monterey=2022) due to some reason. I was hoping this is not the problem, but it seems this is a part of problem.
I propose to close this. For now, my workaround is okay for myself, and good enough to finish a PR in PyMFEM (mfem/PyMFEM#254). I will see if I can update MacOS.

@pazner
Copy link
Member

pazner commented Oct 22, 2024

Thanks @sshiraiwa. Closing for now. Please re-open if it continues to be an issue.

@pazner pazner closed this as completed Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants