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

MathMore and optional features #64

Closed
xkzl opened this issue Dec 12, 2023 · 15 comments
Closed

MathMore and optional features #64

xkzl opened this issue Dec 12, 2023 · 15 comments

Comments

@xkzl
Copy link

xkzl commented Dec 12, 2023

Hi,

I have been using root-project/root:latest docker image, but mathmore and many features are missing.
Is there a prebuilt image including all features available or this is only minimal root ?

=> [17/18] RUN root-config --features
 => => # cxx17 asimage builtin_clang builtin_cling builtin_llvm builtin_openui5 builtin_vdt clad dataframe test_distrdf_pyspark test_distrdf_dask davix fitsio gdml http mlp minuit2 mysql
 => => #  opengl pgsql proof pyroot roofit webgui root7 rpath runtime_cxxmodules shared sqlite ssl tmva tmva-pymva spectrum vdt x11 xml xrootd                                            

Cheers, M.

@xkzl
Copy link
Author

xkzl commented Dec 12, 2023

Additionnally compiling by hand and enabling -Dall=on during cmake step results in:

134.1 Running /opt/root-6-30-03/build/unix/compiledata.sh
134.1 Making /opt/rootbuild/ginclude/compiledata.h
134.2 -- ROOT Configuration 
134.2 
134.2 System          Linux-5.15.49-linuxkit-pr
134.2 Processor       1 core 0 MHz Unknown family (x86_64)
134.2 Build type      Release
134.2 Install path    /usr/local
134.2 Compiler        GNU 11.4.0
134.2 Compiler flags:
134.2 C                -Wno-implicit-fallthrough -pipe -Wall -W -pthread -O3 -DNDEBUG
134.2 C++              -std=c++17 -Wno-implicit-fallthrough -Wno-noexcept-type -pipe  -Wshadow -Wall -W -Woverloaded-virtual -fsigned-char -pthread -O3 -DNDEBUG
134.2 Linker flags:
134.2 Executable       -rdynamic
134.2 Module          
134.2 Shared           -Wl,--no-undefined -Wl,--hash-style="both"
134.2 
134.2 -- Enabled support for:  asimage builtin_clang builtin_cling builtin_cppzmq builtin_llvm builtin_openui5 builtin_unuran builtin_vdt builtin_veccore builtin_xrootd builtin_zeromq cefweb clad cuda dataframe davix dcache fftw3 fitsio fortran gdml gviz http fcgi imt mathmore mlp mysql odbc opengl pgsql proof pyroot qt5web roofit roofit_multiprocess webgui root7 rpath runtime_cxxmodules shadowpw shared sqlite ssl tmva tmva-cpu tmva-pymva spectrum unuran vdt veccore x11 xml xrootd
134.5 -- Configuring done
135.5 CMake Error at cmake/modules/RootMacros.cmake:910 (add_library):
135.5   Cannot find source file:
135.5 
135.5     AnalyticalIntegrals.cxx
135.5 
135.5   Tried extensions .c .C .c++ .cc .cpp .cxx .cu .mpp .m .M .mm .ixx .cppm .h
135.5   .hh .h++ .hm .hpp .hxx .in .txx .f .F .for .f77 .f90 .f95 .f03 .hip .ispc
135.5 Call Stack (most recent call first):
135.5   cmake/modules/RootMacros.cmake:1350 (ROOT_LINKER_LIBRARY)
135.5   hist/hist/CMakeLists.txt:11 (ROOT_STANDARD_LIBRARY_PACKAGE)
135.5 
135.5 
137.3 CMake Error at cmake/modules/RootMacros.cmake:910 (add_library):
137.3   No SOURCES given to target: Hist
137.3 Call Stack (most recent call first):
137.3   cmake/modules/RootMacros.cmake:1350 (ROOT_LINKER_LIBRARY)
137.3   hist/hist/CMakeLists.txt:11 (ROOT_STANDARD_LIBRARY_PACKAGE)
137.3 
137.3 
137.3 CMake Generate step failed.  Build files cannot be regenerated correctly.
------

@chrisburr
Copy link
Member

Is there anthing you're specficially after? The conda based root images have more features enabled.

@xkzl
Copy link
Author

xkzl commented Dec 12, 2023

I specifically want to use docker to include root in some of my developments with a Dockerfile. About ROOT, I specifically need mathmore and gsl mainly.

I have manage to compile ROOT with minimal cmake options including these mathmore and libgsl0-dev.

However, following this. There is an issue about GSL component detection.
Do you think it is this related to this recent issues about ? (vdt & json detection?)

6.635 CMake Error at /opt/root/cmake/ROOTConfig.cmake:166 (message):
6.635   ROOT component GSL not found
6.635 Call Stack (most recent call first):
6.635   lib/UIUC/cmake/Inc/LoadRequirements.cmake:5 (find_package)
6.635   lib/UIUC/CMakeLists.txt:26 (include)

Would you have any idea how to fix ?

@ferdymercury
Copy link

hmm maybe you are missing the GSLConfig.cmake file in your system. What does 'locate GSLConfig.cmake' print? If it exists, then maybe adding it to cmake prefix path would help.
See also: microsoft/GSL#715

@xkzl
Copy link
Author

xkzl commented Dec 12, 2023

I have checked and "find_package(GSL REQUIRED)" is working fine it can locate GSL.
It looks like an issue within ROOTConfig.cmake.

The fix I found is to remove GSL from the ROOT component still require Matrix (that is using it) and then load GSL on my own.

But this looks very different compared to what I was doing in the past. I never had to load GSL by myself while using Polynomial or Matrix classes

@ferdymercury
Copy link

Maybe @krasznaa has some ideas where the bug might come from

@krasznaa
Copy link

Wait... 😕 I think it's your code @xkzl that is doing something funky here. 🤔

The error that you quoted is printed by the code that is checking whether all the ROOT components that you specified to

find_package(ROOT REQUIRED COMPONENTS ...)

are part of ROOT.

But GSL is not a component in ROOT. It is "just" something that ROOT itself privately depends on. Since the dependency is private, ROOTConfig.cmake doesn't need to find GSL in any shape or form to make you able to use the ROOT header/libraries.

So, you should just not be listing "GSL" as a required component when searching for ROOT.

Or did I misunderstand the issue completely? 😕

@ferdymercury
Copy link

Or did I misunderstand the issue completely? 😕

I think that the problem is that:

find_package(ROOT REQUIRED COMPONENTS Matrix)

leads later to an error when trying to run the user script. He has to add in the script gSystem->Load("libGSL").

So there is a problem with the runpath library or something like that.

Or he has to call separately find_package(GSL) and add GSL_LIBRARIES while calling target_link_libraries

Before he didn't need to do anything like that, the private internal dependencies where automatically taken care of. So it's not detecting well the builtin and one has to install the system-wide. At least that's what I understood from his message.

@ferdymercury
Copy link

Maybe related: root-project/root#8709

@xkzl
Copy link
Author

xkzl commented Dec 17, 2023

Hi ppl,

I agree, that my initial issue appeared when adding Matrix as root compoennt.
I noticed that there were these issue at runtime due to GSL, so I tried to figure out and noticed that root is actually using it.
Adding GSL in the list of component appeared to fix the issue with ROOT 6.29 in my local distribution (but obviously not) however I could compile and run and I through this was eventually the proper way to load GSL as it was internally built.

In the recent version of ROOT I had to add find_package(GSL) in order to compile my root library to be used with my root programs, so perhap it is related to @ferdymercury suggestion.

Right now it is working and my problem is fixed by adding manually GSL, but it doesnt sounds natural.
GSL is built internally and I am just using an internal features.

@xkzl
Copy link
Author

xkzl commented Dec 17, 2023

About the mathmore features, I just enabled partial features and recompiled ROOT in my docker component.
So this is also fixed.

@krasznaa
Copy link

Hi,

Interesting. But let's look at things in order.

  • MathMore does only depend on GSL headers "privately" based on a quick git grep, so it's fine that ROOTConfig.cmake doesn't look for GSL actively for MathMore's sake.
  • There does seem to be a single public include of GSL in:

https://github.com/root-project/root/blob/master/tmva/tmva/inc/TMVA/DNN/Architectures/Cpu/Blas.h#L58

Yet GSL is only linked privately against the TMVA library. 😦

https://github.com/root-project/root/blob/master/tmva/tmva/CMakeLists.txt#L419-L431

This all looks very similar to the thing that I fixed with #11844. 🤔

  • For private dependencies, while CMake does not need to be aware of them, your linker does! As a simple example, let's say that ROOT::Foo privately depends on library libBar.so. Now, when you want to make your own code use ROOT::Foo, you just declare to CMake that you want to use that library. And CMake also doesn't bother telling anything more to the linker, since ROOT::Foo only privately depended on libBar.so.

But the linker in most situations (this is another longer story...) does need to find libBar.so while linking your code. Since even for "private" dependencies libROOTFoo.so will still declare that it needs libBar.so to function. So the linker does need to find libBar.so.

In most cases you achieve this by making sure that libBar.so is in $LD_LIBRARY_PATH. (Or is in any other way visible to the system.)

So, to summarize:

  • I believe ROOT does have a configuration bug in TMVA, but that's not what affects you.
  • You do need to make sure that any private dependency of ROOT would be in your $LD_LIBRARY_PATH. As ROOT is correct in not disclosing such dependencies through CMake.

This private dependency linking business is a messy thing for sure. 😦 We've had to deal with this in the ATLAS offline software build since forever... 🤔

@xkzl
Copy link
Author

xkzl commented Dec 18, 2023

I will try to make a minimal reproducer. Thank you so much for your detailed explanation! Very helpful.

@ferdymercury
Copy link

TMVA bug will be fixed by root-project/root#14330

@ferdymercury
Copy link

ferdymercury commented Feb 1, 2024

@xkzl please close if the compilation issues you encountered are fixed by 6.30.04 ;)

Maybe you also need to cherry-pick this: root-project/root#14330

@xkzl xkzl closed this as completed Feb 5, 2024
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

4 participants