umd-lhcb / root-curated

A curated software root (foundation) for most of our analysis repos
BSD 2-Clause "Simplified" License
3 stars 0 forks source link

Test if ROOT 6.16 branch works with Phoebe's patch #5

Closed yipengsun closed 3 years ago

yipengsun commented 3 years ago

So far we have ROOT 6.12 branch working, but it won't compile with Python 3. We should still try to find a more recent ROOT version.

Here let's try updating our ROOT derivations to 6.18 and apply Phoebe's patch to see if it works.

yipengsun commented 3 years ago

@manuelfs Can you try to create a ROOT 6.18.04 derivation based on this instruction?

manuelfs commented 3 years ago

Is that the first version that supports python 3? I ask because it may be better to set up ROOT 6.16/00 which we know works with HAMMER from RD+.

Also, from those instructions is not clear where the patches go in. I assume I just copy the github diff into the hist_factory.patch file?

yipengsun commented 3 years ago

HAMMER seems to require C++ 14: https://hammer.physics.lbl.gov/readme.html

We probably should compile our ROOT with at least C++ 14.

manuelfs commented 3 years ago

I started following these instructions to add a ROOT 6.16/00 derivation, but I don't know how to test if it builds. It fails both for root_6_16 and root_6_12 so I must be doing something wrong

(base) |16:56:50|~/code/root-curated$ nix build ".#root_6_16_00"
warning: Git tree '/Users/manuelf/code/root-curated' is dirty
error: getting status of '/nix/store/dmi6ypn1fypmshwj0iryajs3zsm3dmwa-source/nix/root_6_16': No such file or directory
(use '--show-trace' to show detailed location information)
(base) |16:58:02|~/code/root-curated$ nix build ".#root_6_16"
warning: Git tree '/Users/manuelf/code/root-curated' is dirty
error: getting status of '/nix/store/dmi6ypn1fypmshwj0iryajs3zsm3dmwa-source/nix/root_6_16': No such file or directory
(use '--show-trace' to show detailed location information)
(base) |16:58:20|~/code/root-curated$ nix build ".#root_6_12"
warning: Git tree '/Users/manuelf/code/root-curated' is dirty
error: getting status of '/nix/store/dmi6ypn1fypmshwj0iryajs3zsm3dmwa-source/nix/root_6_16': No such file or directory
(use '--show-trace' to show detailed location information)
(base) |16:59:04|~/code/root-curated$ nix build ".#root_6_12_06"
warning: Git tree '/Users/manuelf/code/root-curated' is dirty
error: getting status of '/nix/store/dmi6ypn1fypmshwj0iryajs3zsm3dmwa-source/nix/root_6_16': No such file or directory
(use '--show-trace' to show detailed location information)
manuelfs commented 3 years ago

Committed the code I have so far to a new branch https://github.com/umd-lhcb/root-curated/tree/test_root_6_16

yipengsun commented 3 years ago

You can test the build with:

nix build ".#root_6_16_00"

And I get this error:

error: builder for '/nix/store/xk94sw6rf9kgkzxibgbiib0fpg9q3378-root-6.16.00.drv' failed with exit code 1;
       last 10 log lines:
       > source root is root-6.16.00
       > setting SOURCE_DATE_EPOCH to timestamp 1548234445 of file root-6.16.00/etc/gitinfo.txt
       > patching sources
       > applying patch /nix/store/2j87bvmighrk8m0b000av56m0dh3pcns-sw_vers.patch
       > patching file build/unix/compiledata.sh
       > patching file cmake/modules/SetUpMacOS.cmake
       > Hunk #3 FAILED at 100.
       > 1 out of 3 hunks FAILED -- saving rejects to file cmake/modules/SetUpMacOS.cmake.rej
       > patching file config/root-config.in
       > Hunk #1 succeeded at 307 (offset 3 lines).
       For full logs, run 'nix log /nix/store/xk94sw6rf9kgkzxibgbiib0fpg9q3378-root-6.16.00.drv'.

This is actually not too bad, as the failed patch is for patching build systems (the CMake files). I'll first try if the CMake patch for 6.18 works for 6.16. If not, I'll port the CMake changes for 6.16.

yipengsun commented 3 years ago

I think the mistake you made was:

nix build ".#root_6_16"

This won't work, as both in overlay.nix and flake.nix, we name the package as root_6_16_00 instead of root_6_16. Don't confuse folder name with package names :-)

yipengsun commented 3 years ago

I updated the patch, and I am compiling it on both my laptop and Quan's MacBook w/ Big Sur. You can try it on Catalina too.

yipengsun commented 3 years ago

Both builds error out with different but related error messages :-(

yipengsun commented 3 years ago

On Linux, the error messages are:

[ 74%] Generating G__Core.cxx, ../../lib/libCore.rootmap
In file included from input_line_9:150:
/build/root-6.16.00/build/include/ROOT/RIntegerSequence.hxx:89:8: error: redefinition of 'integer_sequence'
struct integer_sequence {
       ^
/nix/store/bqgh2vm981wscyqhvv0fyzdih52hwc7n-gcc-10.3.0/lib/gcc/x86_64-unknown-linux-gnu/10.3.0/../../../../include/c++/10.3.0/utility:326:12: note: previous definition is here
    struct integer_sequence
           ^
In file included from input_line_9:150:
/build/root-6.16.00/build/include/ROOT/RIntegerSequence.hxx:115:7: error: type alias template redefinition with different types ('__make_integer_sequence<_Tp, _Np>' (aka 'typename __make_integer_sequence_checked<type-parameter-0-0, _Ep>::type') vs '__make_integer_seq<integer_sequence, _Tp, _Num>')
using make_integer_sequence = __make_integer_sequence<_Tp, _Np>;
      ^
/nix/store/bqgh2vm981wscyqhvv0fyzdih52hwc7n-gcc-10.3.0/lib/gcc/x86_64-unknown-linux-gnu/10.3.0/../../../../include/c++/10.3.0/utility:334:11: note: previous definition is here
    using make_integer_sequence
          ^
Error: /build/root-6.16.00/build/core/rootcling_stage1/src/rootcling_stage1: compilation failure (/build/root-6.16.00/build/lib/libCoref2fcdd22c7_dictUmbrella.h)
make[2]: *** [core/base/CMakeFiles/G__Core.dir/build.make:470: core/base/G__Core.cxx] Error 1
make[1]: *** [CMakeFiles/Makefile2:18565: core/base/CMakeFiles/G__Core.dir/all] Error 2
make: *** [Makefile:171: all] Error 2

It's basically the same thing on macOS, difference being error reportage differences between gcc and clang. I think this error is due to the fact that ROOT tries to backport features from C++ standard library and somehow they are not properly turned off and introduced a conflict.

manuelfs commented 3 years ago

I pulled and it now does try to build with nix build ".#root_6_16_00", failing with a similar error

(base) |18:47:03|~/code/root-curated$ nix build ".#root_6_16_00"
error: builder for '/nix/store/wg9a5axpwv3q4raljfcnxwf0q2s7fmhz-root-6.16.00.drv' failed with exit code 2;
       last 10 log lines:
       > /tmp/nix-build-root-6.16.00.drv-0/root-6.16.00/build/include/ROOT/RIntegerSequence.hxx:121:28: error: no template named 'make_index_sequence'; did you mean 'make_integer_sequence'?
       > using index_sequence_for = make_index_sequence<sizeof...(_Tp)>;
       >                            ^
       > /nix/store/bcaknchzdrazlmscllxkvivys6z2p0ks-libcxx-7.1.0-dev/include/c++/v1/utility:883:5: note: 'make_integer_sequence' declared here
       >     using make_integer_sequence = __make_integer_sequence<_Tp, _Np>;
       >     ^
       > Error: /tmp/nix-build-root-6.16.00.drv-0/root-6.16.00/build/core/rootcling_stage1/src/rootcling_stage1: compilation failure (/tmp/nix-build-root-6.16.00.drv-0/root-6.16.00/build/lib/libCore627338001b_dictUmbrella.h)
       > make[2]: *** [core/base/CMakeFiles/G__Core.dir/build.make:472: core/base/G__Core.cxx] Error 1
       > make[1]: *** [CMakeFiles/Makefile2:18648: core/base/CMakeFiles/G__Core.dir/all] Error 2
       > make: *** [Makefile:171: all] Error 2
       For full logs, run 'nix log /nix/store/wg9a5axpwv3q4raljfcnxwf0q2s7fmhz-root-6.16.00.drv'.

It is still not clear to me what was wrong above, as I did try nix build ".#root_6_16_00" (the first of the four in this post). Perhaps that I had tried earlier nix build ".#root_6_16_00"? Though in that case I don't know why a git pull would have cleared it up.

yipengsun commented 3 years ago

That is mysterious indeed. nix build ".#root_6_16_00" should always trigger a build. Could you check your shell history and see what is actually going on?

yipengsun commented 3 years ago

Ah I think I know why:

Previously I mentioned that flake will only consider files that are included in your git repository (a design feature). I think in your initial build, you haven't added the files in git.

yipengsun commented 3 years ago

For new files, git add is sufficient for flake to consider them. You can still edit them as you see fit, and only commit the final version of these files with git commit -a.

yipengsun commented 3 years ago

With my latest patch of enforcing C++ 17 standard, this builds fine both on Linux and macOS Big Sur. @manuelfs Can you try pulling the latest change and see if it builds on Catalina?

I'm going to do some bench mark with Quan's computer, disable ROOT 6.12, and merge your branch.

manuelfs commented 3 years ago

Previously I mentioned that flake will only consider files that are included in your git repository (a design feature). I think in your initial build, you haven't added the files in git.

Ah, that explains it! We should add to the README that the files need to be git added before building.

Pulled the changes and complained about attribute 'root_6_12_06' missing. Commenting that line out in flake.nix, it has started compiling.

yipengsun commented 3 years ago

I've made required changes in histfactory-fitter-demo repo and if you reset your local change and pull mine, it should work. I've tested that ROOT 6.16 works for Big Sur.

I'll update the bench marks once I test ROOT 5.34 as well.

manuelfs commented 3 years ago

I updated the patch, and I am compiling it on both my laptop and Quan's MacBook w/ Big Sur. You can try it on Catalina too.

What did you need to update there?

It now compiles in Catalina. I also updated the README with more clear instructions.

yipengsun commented 3 years ago

The freetype library was missing, resulting in a compilation error of the fitter. So I made that available in the devShell too and it worked for Big Sur.

manuelfs commented 3 years ago

Tested that the HistFactory demo works in ROOT 6.16/00 with Catalina, converged to the usual value in fit ran in 72.711669 seconds with 94.779205 seconds in prep. Updated the benchmarks as well

FCN=-3035.66 FROM MINOS     STATUS=SUCCESSFUL    208 CALLS        1212 TOTAL
                     EDM=0.000346097    STRATEGY= 2      ERROR MATRIX ACCURATE 
  EXT PARAMETER                  PARABOLIC         MINOS ERRORS        
  NO.   NAME      VALUE            ERROR      NEGATIVE      POSITIVE   
   1  Lumi         9.39131e-01   3.53587e-02                            
   2  Nmu          6.37709e+04   4.35719e+02                            
   3  RawRDst      4.05137e-02   4.39518e-03  -4.39678e-03   4.54755e-03