Remove swapping of nodes and stick to merging of same nodes in the same layer for different graphs.

Should the GraphManager split selections by default? What is the impact of doing so?

As it was noticed during the last benchmark tests run, the treatment of unbalanced datasets is suboptimal when running with multiprocessing enabled if one of the RDataFrames is built on top of a dataset whose size is much bigger than the others, the worker that process it end up creating a bottleneck for the entire analysis. Several ways (to be investigated and implemented separately) can fix this issue:

• combine the usage of multiprocessing and multithreading: detect in advance the larger datasets and split the workers that get to process these into multiple threads; in order not to increase the number of cores used, the overall number of workers decreases;
• using only multiprocessing: detect in advance the larger datasets and split them into different RDataFrames, so that they are taken by different workers; the results can be easily merged at the end to get the proper histograms; this solution also requires something to check that the largest RDataFrames are the first ones sent to the workers.

Put a time measurement in the _run_multiprocess function and return the time. this should then be available for the user to see how imbalanced the runs are. also, add a debug message in the _run_multiprocess function for that purpose. that would clear up many cases of "bad scaling".

If I see it correctly we have all changes that are in the abcd_method branch also in the dev branch that I would like to merge into the master branch to keep a clean setup of branches.

This would mean the abcd_method branch is obsolete and could be removed to clean up stale branches we don't plan to develop further anyway. In the past, we often opened branches and then never merged them into the main branch.

Could someone please check again if the code from the abcd_method branch is already included in the dev branch and comment then here. I would then delete the abcd_method branch.

A function run_on_condor could be implemented with htmap, a module that wraps the process of mapping Python function calls out to an HTCondor Pool.

Needs to be better investigated and eventually implemented and tested.

Otherwise throw warning.

Please remove the build_info from the Dataset class. It makes life much more complicated:

https://github.com/KIT-CMS/ntuple_processor/blob/master/utils/_booking.py#L70

I think you introduced it to have the ChangedDataset variation, is this correct? Why can't you just loop over the NTuples there and fiddle around with the info?

The CMS module for the production of stacked histograms needs to be included

This makes you dependent on the artus output structure, but it should not be like this:

https://github.com/KIT-CMS/ntuple_processor/blob/master/variations.py#L21

Please remove this together with the build_info and modify the raw info from the NTuples (see #20).

We could use hashes a tags appended to the names of the results. Each has is unique for each path we go in the computation graphs, which means we can identify results with the same name but different paths. This should then throw a warning.

Could you explain what https://github.com/KIT-CMS/ntuple_processor/blob/master/utils/_booking.py#L53 is doing? Also, we do not use the tags at all, right?

In case I'm right, just remove the Friend class and just use the NTuple for it. There's no real need to sublcass it again. Then, you can also remove the NTupleBase and just use NTuple for everything.

https://github.com/KIT-CMS/ntuple_processor/blob/master/utils/_booking.py#L254

• Please introduce a Binning object, that can be reused.
• The should be only one way to book a Histogram, please remove the conditional setup in the ctor.
• Just use bin edges for the binning, no other mode is required (like the typical nbins, lower, upper mode in ROOT)
• The binning needs a name so that we can build a proper tag for the histogram in the file.

Running a simple analysis after sourcing /home/wunsch/workspace/root/build_/bin/root (self compiled ROOT master) prints
ReadStreamerInfo, class:string, illegal uid=-2

Running the same analysis after sourcing
/cvmfs/sft-nightlies.cern.ch/lcg/views/dev3python3/latest/x86_64-centos7-gcc8-opt/setup.sh
BREAKS.

Why do we need as input of the ReplaceCut class a Cut object? Wouldn't a tuple like (name, new_cut) be enough? That would be in line with the selection class.

When i attempt a lvl 2 optimization, I get an error message

Traceback (most recent call last):
  File "/work/sbrommer/smhtt_ul/tauID/smhtt_ul/shapes/produce_shapes_tauID.py", line 1071, in <module>
    main(args)
  File "/work/sbrommer/smhtt_ul/tauID/smhtt_ul/shapes/produce_shapes_tauID.py", line 1036, in main
    g_manager.optimize(args.optimization_level)
  File "/work/sbrommer/smhtt_ul/tauID/smhtt_ul/ntuple_processor/optimization.py", line 116, in optimize
    self.optimize_selections()
  File "/work/sbrommer/smhtt_ul/tauID/smhtt_ul/ntuple_processor/optimization.py", line 140, in optimize_selections
    self._merge_children(merged_graph)
  File "/work/sbrommer/smhtt_ul/tauID/smhtt_ul/ntuple_processor/optimization.py", line 165, in _merge_children
    self._merge_children(child)
  File "/work/sbrommer/smhtt_ul/tauID/smhtt_ul/ntuple_processor/optimization.py", line 165, in _merge_children
    self._merge_children(child)
  File "/work/sbrommer/smhtt_ul/tauID/smhtt_ul/ntuple_processor/optimization.py", line 165, in _merge_children
    self._merge_children(child)
  [Previous line repeated 18 more times]
  File "/work/sbrommer/smhtt_ul/tauID/smhtt_ul/ntuple_processor/optimization.py", line 157, in _merge_children
    if child not in merged_children:
ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all()

unclear how this happens.

Try to perform the I/O part with Uproot https://github.com/scikit-hep/uproot4 and check whether there is a speed improvement, and how much it is. If this is huge, it is probably worth inserting a dependency in order to gain something in performance.

I'm missing basic error handling in the variations, e.g., if you want to use ReplaceCut, then you should throw an error if the requested cut is not found. Same goes for all other classes.

It probably makes more sense to have https://github.com/KIT-CMS/ntuple_testsuite as a subdirectory of ntuple_processor instead of ntuple-analysis, since it tests only the methods of the core machine.

The sample validation of the crown inputs yields an output that looks like the check has successfully passed even though it did not run all. This happens because an empty dict is created and not filled when no check is required but the dict is then used for the printout independent of the requirement of the sample validation.
I think we should improve on this and make the output also conditional.

Consider if it is worth making a Python package for this
see https://packaging.python.org/tutorials/packaging-projects/