Parallel version of Snapshot in TDataFrame using TBufferMerger

[amadio]

Passes test_snapshot.C on my computer. However, I believe there are problems elsewhere when more than one cluster exists in the generated trees (i.e. when ForeachSlot actually has more than one slot). We need to make sure to test TDataFrame with trees with multiple slots in the future.

[phsft-bot]

Starting build on gcc49/centos7, native/mac1012, gcc49/slc6, gcc62/slc6, native/ubuntu14 with CMake flags -Dvc=OFF -Dimt=ON -Dccache=ON

[amadio]

For example, the script below sometimes fails because the Mean results are different due to the different ordering of the data. Setting the auto flush to a large number then should get rid of the problem because then there will be only a single cluster in the input tree.

#include "TFile.h"
#include "TH1F.h"
#include "TTree.h"

#include <iostream>
#include <random>

#include "ROOT/TDataFrame.hxx"

void fill_tree(const char *filename, const char *treeName)
{
   TFile f(filename, "RECREATE");
   TTree t(treeName, treeName);

   t.SetAutoFlush(64); // for creation of clusters to test in MT mode

   int b1;
   float b2;
   t.Branch("b1", &b1);
   t.Branch("b2", &b2);
   for (int i = 0; i < 1024 * 64; ++i) {
      b1 = i;
      b2 = std::rand();
      t.Fill();
   }
   t.Write();
   f.Close();
   return;
}

class A {
public:
   A(){};
   A(int i):fI(i){}
   int GetI(){return fI;}
private:
   int fI = 0;
};

int test_snapshot()
{
   ROOT::EnableImplicitMT();

   auto fileName = "test_snapshot.root";
   auto outFileName = "test_snapshot_output.root";
   auto treeName = "myTree";

   fill_tree(fileName, treeName);

   ROOT::Experimental::TDataFrame d(treeName, fileName);

   auto d_cut = d.Filter("b1 % 2 == 0");

   auto d2 = d_cut.Define("a","A(b1)")
                  .Define("b1_square", "b1 * b1")
                  .Define("b2_vector", [](float b2){ std::vector<float> v; for (int i=0;i < 3; i++) v.push_back(b2*i); return v;}, {"b2"});

   auto snapshot_tdf = d2.Snapshot<int, int, std::vector<float>, A>(treeName, outFileName, {"b1", "b1_square", "b2_vector", "a"});

   // Open the new file and list the branches of the tree
   TFile f(outFileName);
   TTree* t;
   f.GetObject(treeName, t);
   for (auto branch : *t->GetListOfBranches()) {
      std::cout << "Branch: " << branch->GetName() << std::endl;
   }
   f.Close();

   auto max_b1 = snapshot_tdf.Max("b1");
   auto max_a = snapshot_tdf.Define("a_val",[](A& a){return a.GetI();},{"a"}).Max("a_val");

   std::cout << "Maximum: max(b1) = " << *max_b1 << ", max(A(b1)) = " << *max_a << std::endl;

   if (*max_b1 != *max_a) {
      std::cerr << "Error: the maximum values of two branches which are supposed to be identical differ!\n";
      return 1;
   }

   return 0;
}

[pcanal]

should'nt it be SafeDelete(fFile);
i.e. is the object pointed to by fFile re-used after this statement?

[amadio]

No, fFile is owned by the TBufferMerger class, it's a unique_ptr. It gets destroyed with the server. I just noticed, however, that the documentation on the header is wrong, I will update it.

[pcanal]

I am not 'finding' the pointer where the shared_ptr are copied/shared. Wouldn't this work the same with unique_ptr?

[pcanal]

[As a side note since ownership is 'implicitly' shared with the TFile. A raw pointer here would work too].

[amadio]

The files need to be shared, because the server returns std::shared_ptr and keeps a weak_ptr to them to check they are expired when the server goes down. The TTrees can probably be unique_ptr, but there is not std::make_unique in C++11, only make_shared.

[pcanal]

but there is not std::make_unique in C++11
So? you can still make unique_ptr, the syntax is just slightly longer (unique_ptr(new X(...))

[amadio]

Ok, I will make it unique_ptr, although I don't think there is that big of an advantage over shared_ptr here.

[amadio]

Side note, should we just backport std::make_unique to be able to use it now? If string_view was backported, I think backporting std::make_unique would be even more useful to write better code.

[amadio]

Done.

[pcanal]

Side note, should we just backport std::make_unique to be able to use it now? If string_view was backported, I think backporting std::make_unique would be even more useful to write better code.

@Axel-Naumann What's is your opinion on this?

[phsft-bot]

Starting build on gcc49/centos7, native/mac1012, gcc49/slc6, gcc62/slc6, native/ubuntu14 with CMake flags -Dvc=OFF -Dimt=ON -Dccache=ON

[phsft-bot]

Build failed on mac1012/native.
See console output.

Errors:

• clang: error: linker command failed with exit code 1 (use -v to see invocation)

[amadio]

Failed with 13:53:02 FAILED: lib/libOracle.so. Is master broken?

[phsft-bot]

Starting build on gcc49/centos7, native/mac1012, gcc49/slc6, gcc62/slc6, native/ubuntu14 with CMake flags -Dvc=OFF -Dimt=ON -Dccache=ON

[phsft-bot]

Build failed on mac1012/native.
See console output.

Errors:

• clang: error: linker command failed with exit code 1 (use -v to see invocation)

[phsft-bot]

Build failed on centos7/gcc49.
See console output.

Failing tests:

• projectroot.roottest.root.dataframe.roottest_root_dataframe_regression_snapshot

[amadio]

@dpiparo Thank you for merging. Let's keep track of that failing test, however. It runs on my machine, but there may be an actual problem there due to the different environment.