sparrow icon indicating copy to clipboard operation
sparrow copied to clipboard

Compile notes

Open frobnitzem opened this issue 2 years ago • 5 comments

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 avatar Feb 28 '22 20:02 frobnitzem

  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

frobnitzem avatar Feb 28 '22 23:02 frobnitzem

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.

weymutht avatar Mar 01 '22 06:03 weymutht

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

https://github.com/spack/spack/pull/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!

frobnitzem avatar Mar 02 '22 14:03 frobnitzem

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

awvwgk avatar Apr 09 '22 14:04 awvwgk

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.

frobnitzem avatar Sep 20 '22 21:09 frobnitzem