search

qcscine / sparrow

https://scine.ethz.ch
BSD 3-Clause "New" or "Revised" License
78 stars 15 forks source link

Compile notes #6

Open frobnitzem opened 2 years ago

frobnitzem commented 2 years ago

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_perbohr * 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_perbohr * 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_perbohr * exp(-pA.alpha() * radiusDeriv) + ^ detected during: instantiation of "Scine::Utils::AutomaticDifferentiation::Value1DType Scine::Sparrow::nddo::AM1PairwiseRepulsion::standardParenthesis(double) const [with O=Scine::Utils::DerivativeOrder::Zero]" at line 77 instantiation of "Scine::Utils::AutomaticDifferentiation::Value1DType Scine::Sparrow::nddo::AM1PairwiseRepulsion::parenthesisValue(double) const [with O=Scine::Utils::DerivativeOrder::Zero]" at line 72 instantiation of "Scine::Utils::AutomaticDifferentiation::Value1DType Scine::Sparrow::nddo::AM1PairwiseRepulsion::baseTerm(double) const [with O=Scine::Utils::DerivativeOrder::Zero]" at line 66 instantiation of "Scine::Utils::AutomaticDifferentiation::Value1DType Scine::Sparrow::nddo::AM1PairwiseRepulsion::calculateRepulsion(double) const [with O=Scine::Utils::DerivativeOrder::Zero]" at line 23 of "~/sparrow/src/Sparrow/Sparrow/Implementations/Nddo/Am1/AM1PairwiseRepulsion.cpp"


2. 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](https://cmake.org/cmake/help/latest/prop_tgt/CXX_STANDARD.html).

3. 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 commented 2 years ago
  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 commented 2 years ago

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 commented 2 years ago

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!

awvwgk commented 2 years ago

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 commented 1 year ago

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.