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

Compile notes #6

Open
frobnitzem opened this issue Feb 28, 2022 · 5 comments
Open

Compile notes #6

frobnitzem opened this issue Feb 28, 2022 · 5 comments

Comments

@frobnitzem
Copy link

Thanks for providing this package!

I want to try out molgym using this great semiempirical package, and noticed the following build issues:

  1. compiling with intel/19.0.3 leads to confusion about template arguments:
$ cmake -DCMAKE_BUILD_TYPE=Release -DSCINE_BUILD_PYTHON_BINDINGS=ON -DCMAKE_INSTALL_PREFIX=$INST -DCMAKE_CXX_COMPILER=`which icc` -DCMAKE_C_COMPILER=`which icc` ..

$ make
...

~/sparrow/src/Sparrow/Sparrow/Implementations/Nddo/Am1/AM1PairwiseRepulsion.h(91): error: no instance of overloaded function "exp" matches the argument list
            argument types are: (double)
      return res + radiusDeriv * Utils::Constants::angstrom_per_bohr * exp(-pA_.alpha() * radiusDeriv) +
                                                                       ^
~/sparrow/src/Sparrow/Sparrow/Implementations/Nddo/Am1/AM1PairwiseRepulsion.h(91): note: this candidate was rejected because arguments do not match
      return res + radiusDeriv * Utils::Constants::angstrom_per_bohr * exp(-pA_.alpha() * radiusDeriv) +
                                                                       ^
~/sparrow/src/Sparrow/Sparrow/Implementations/Nddo/Am1/AM1PairwiseRepulsion.h(91): note: this candidate was rejected because at least one template argument could not be deduced
      return res + radiusDeriv * Utils::Constants::angstrom_per_bohr * exp(-pA_.alpha() * radiusDeriv) +
                                                                       ^
~/sparrow/src/Sparrow/Sparrow/Implementations/Nddo/Am1/AM1PairwiseRepulsion.h(91): note: this candidate was rejected because arguments do not match
      return res + radiusDeriv * Utils::Constants::angstrom_per_bohr * exp(-pA_.alpha() * radiusDeriv) +
                                                                       ^
          detected during:
            instantiation of "Scine::Utils::AutomaticDifferentiation::Value1DType<O> Scine::Sparrow::nddo::AM1PairwiseRepulsion::standardParenthesis<O>(double) const [with O=Scine::Utils::DerivativeOrder::Zero]" at line 77
            instantiation of "Scine::Utils::AutomaticDifferentiation::Value1DType<O> Scine::Sparrow::nddo::AM1PairwiseRepulsion::parenthesisValue<O>(double) const [with O=Scine::Utils::DerivativeOrder::Zero]" at line 72
            instantiation of "Scine::Utils::AutomaticDifferentiation::Value1DType<O> Scine::Sparrow::nddo::AM1PairwiseRepulsion::baseTerm<O>(double) const [with O=Scine::Utils::DerivativeOrder::Zero]" at line 66
            instantiation of "Scine::Utils::AutomaticDifferentiation::Value1DType<O> Scine::Sparrow::nddo::AM1PairwiseRepulsion::calculateRepulsion<O>(double) const [with O=Scine::Utils::DerivativeOrder::Zero]" at line 23 of "~/sparrow/src/Sparrow/Sparrow/Implementations/Nddo/Am1/AM1PairwiseRepulsion.cpp"
  1. Compiling on OSX with stock clang mostly succeeds, but gives lots of warnings about C++17-style constants
$ clang++ --version
Apple clang version 13.0.0 (clang-1300.0.29.30)
Target: x86_64-apple-darwin20.6.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin

src/Sparrow/Sparrow/Resources/Dftb/3ob-3-1/Parameters.cpp:19443:10: warning: hexadecimal floating literals are a C++17 feature [-Wc++17-extensions]
        {0x1.f333333333333p+2, 0x1.f666666666666p+2, 0x1.4c345fa238401p-23, -0x1.9f461446d6afap-19, 0x1.812b43ac0c844p-16, -0x1.1b403de0c351dp-14}, 
         ^

consider setting CMAKE_CXX_STANDARD.

  1. OSX filenames are case-insensitive, so the last compile step dies:
[ 89%] Linking CXX executable sparrow
cd ~/spack-src/src/Sparrow && cmake -E cmake_link_script CMakeFiles/SparrowApp.dir/link.txt --verbose=1
/usr/local/spack/lib/spack/env/clang/clang++  ... -o sparrow  ...

ld: can't write output file to 'sparrow' because that path is a directory
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [src/Sparrow/sparrow] Error 1
make[1]: *** [src/Sparrow/CMakeFiles/SparrowApp.dir/all] Error 2
make: *** [all] Error 2
@frobnitzem
Copy link
Author

  1. warnings about redeclarations of some classes as structs (core vs. sparrow App options)
/private/var/folders/q7/dxctxdc16yxc8ms51wyxwddrwxqnlb/T/spack-stage/spack-stage-sparrow-3.0.0-7w7p4sj4v7y6zqriqb6krfgoby65bp4x/spack-build-7w7p4sj/scine-core-src/src/Core/Core/Log.h:57:1: warning: 'Log' defined as a struct here but previously declared as a class; this is valid, but may result in linker errors under the Microsoft C++ ABI [-Wmismatched-tags]
struct CORE_EXPORT Log {
^
/var/folders/q7/dxctxdc16yxc8ms51wyxwddrwxqnlb/T/spack-stage/spack-stage-sparrow-3.0.0-7w7p4sj4v7y6zqriqb6krfgoby65bp4x/spack-src/src/Sparrow/App/CommandLineOptions.h:15:1: note: did you mean struct here?
class Log;
^~~~~
struct

/var/folders/q7/dxctxdc16yxc8ms51wyxwddrwxqnlb/T/spack-stage/spack-stage-sparrow-3.0.0-7w7p4sj4v7y6zqriqb6krfgoby65bp4x/spack-src/src/Sparrow/Sparrow/Implementations/Dftb/TimeDependent/LinearResponse/TDDFTBData.h:27:1: warning: 'TDDFTBData' defined as a struct here but previously declared as a class; this is valid, but may result in linker errors under the Microsoft C++ ABI [-Wmismatched-tags]
struct TDDFTBData : public LinearResponseData {
^
/var/folders/q7/dxctxdc16yxc8ms51wyxwddrwxqnlb/T/spack-stage/spack-stage-sparrow-3.0.0-7w7p4sj4v7y6zqriqb6krfgoby65bp4x/spack-src/src/Sparrow/Sparrow/Implementations/Dftb/DFTBMethodWrapper.h:15:1: note: did you mean struct here?
class TDDFTBData;
^~~~~
struct

ftb/TimeDependent/LinearResponse/BasisPruner.h:20:1: warning: class 'Excitation' was previously declared as a struct; this is valid, but may result in linker errors under the Microsoft C++ ABI [-Wmismatched-tags]
class Excitation;
^
/private/var/folders/q7/dxctxdc16yxc8ms51wyxwddrwxqnlb/T/spack-stage/spack-stage-sparrow-3.0.0-7w7p4sj4v7y6zqriqb6krfgoby65bp4x/spack-build-7w7p4sj/scine-utils-os-src/src/Utils/Utils/TimeDependent/TransitionDipoleCalculator.h:25:8: note: previous use is here
struct Excitation {
       ^

/var/folders/q7/dxctxdc16yxc8ms51wyxwddrwxqnlb/T/spack-stage/spack-stage-sparrow-3.0.0-7w7p4sj4v7y6zqriqb6krfgoby65bp4x/spack-src/src/Sparrow/Sparrow/Implementations/Nddo/TimeDependent/LinearResponse/CISData.h:24:1: warning: 'CISData' defined as a struct here but previously declared as a class; this is valid, but may result in linker errors under the Microsoft C++ ABI [-Wmismatched-tags]
struct CISData : public LinearResponseData {
^
/var/folders/q7/dxctxdc16yxc8ms51wyxwddrwxqnlb/T/spack-stage/spack-stage-sparrow-3.0.0-7w7p4sj4v7y6zqriqb6krfgoby65bp4x/spack-src/src/Sparrow/Sparrow/Implementations/Nddo/NDDOMethodWrapper.h:23:1: note: did you mean struct here?
class CISData;
^~~~~
struct

@weymutht
Copy link
Member

weymutht commented Mar 1, 2022

Thanks a lot for reporting these issues! We will try to accommodate as many of them as possible in the next release. However, please note that we are unable to provide support for mac OSX as we don't use this operating system ourselves.

@frobnitzem
Copy link
Author

By the way, I've created a spack package for easy installation of sparrow from source.

spack/spack#29291

I had to put in 2 patches - one to change the SparrowApp target to output "sparrow.exe" (which is renamed back to sparrow after installation), and another to fix the C++ standard level. With those, I can confirm it compiles with both clang-13.0 and gcc-10.3!

@awvwgk
Copy link

awvwgk commented Apr 9, 2022

Here is the patch I'm using on OSX:

diff --git a/src/Sparrow/CMakeLists.txt b/src/Sparrow/CMakeLists.txt
index 3d67744..6318e03 100644
--- a/src/Sparrow/CMakeLists.txt
+++ b/src/Sparrow/CMakeLists.txt
@@ -120,6 +120,7 @@ add_executable(SparrowApp ${SPARROW_APP_FILES})
 add_executable(Scine::SparrowApp ALIAS SparrowApp)
 
 set_target_properties(SparrowApp PROPERTIES OUTPUT_NAME sparrow)
+set_target_properties(SparrowApp PROPERTIES RUNTIME_OUTPUT_DIRECTORY ${PROJECT_BINARY_DIR})
 
 target_link_libraries(SparrowApp PRIVATE
   Boost::program_options

@frobnitzem
Copy link
Author

In release 3.1.0, compiling using gcc/10.3 works almost out of the box for me. I still get an error on std::transform though. I had to fix it with:

diff --git a/src/Sparrow/Embed/CMakeLists.txt b/src/Sparrow/Embed/CMakeLists.txt
index 01acef0..f2fa135 100644
--- a/src/Sparrow/Embed/CMakeLists.txt
+++ b/src/Sparrow/Embed/CMakeLists.txt
@@ -5,6 +5,7 @@ add_executable(Embed
   ${CMAKE_CURRENT_SOURCE_DIR}/NddoParameters.cpp
 )
 target_include_directories(Embed PRIVATE ${CMAKE_CURRENT_SOURCE_DIR})
+set_property(TARGET Embed PROPERTY CXX_STANDARD 17)
 target_link_libraries(Embed
   PRIVATE Scine::Sparrow cereal
 )

compiling with Intel compiler shows the same exp(...) type error. It might be possible to fix it by casting the argument, e.g. exp((double) ... ), but I haven't tried it.

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

No branches or pull requests

3 participants