No copy constructor for ROOT::Experimental::THist?

Hi,

I was trying to play a bit with the experimental histogram class(es) in 6.12.04. But I found quickly that the following doesn’t work. :frowning:

using ROOT::Experimental::TH1D;
using ROOT::Experimental::TAxisConfig;

std::vector< TH1D > hists( 10, TH1D( TAxisConfig( 100, -3.0, 3.0 ) ) );

Since apparently ROOT::Experimental::THist doesn’t have a copy constructor.

In file included from /Users/krasznaa/Development/ROOT/root7HistTests/source/histogramTest.cxx:3:
In file included from /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/vector:266:
In file included from /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/__bit_reference:15:
In file included from /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/algorithm:640:
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/memory:1783:31: error: 
      call to implicitly-deleted copy constructor of 'ROOT::Experimental::THist<1, double, THistStatContent,
      THistStatUncertainty>'
            ::new((void*)__p) _Up(_VSTD::forward<_Args>(__args)...);
                              ^   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
...

Since according to the build error the copy-constructor is explicitly deleted, I have to wonder: why? I thought (one of) the point(s) of the new histogram class was to make it behave like a “regular object”. Being able to freely copy it would seem like a requirement for that. :confused:

Any insight into this design decision will be greatly appreciated.

Cheers,
Attila

Hi Attila,

I’m torn between allowing “implicit” copies (that are expensive, because you’re coping possibly multiple arrays) and forcing users to call an explicit copy operation. I.e. between “novice-protective” and “unsurprising”. Especially as

auto hist = ::TH1F(...)
hist->Draw();
hist->Fill(1.);

will update the drawn hist, while

ROOT::Experimental::TH1F hist(...);
canv.Draw(hist);
hist.Fill(1.)

will draw a copy of hist (if it had a copy c’tor), and not update the drawn histogram by the Fill().

What are your arguments pro and cons?

Axel.

Hi Axel,

How the histograms should behave wrt. canvases is a good question…

For interactive/analysis use having the histogram be fixed/constant after drawing it to a canvas is absolutely fine. At least I can hardly accept an argument why any analysis code should not be organised with this behaviour in mind. (I have never written histogram plotting code that would’ve expected anything else.)

But compiled applications (like some monitoring application) can absolutely have a much easier time if visible histograms get updated on fills behind the scenes, and they don’t need to worry about updating the visible histograms themselves explicitly.

Is it completely out of the question that drawing would be done by a completely different layer of the code? So that ROOT::THist (once it’s out of the Experimental namespace) is really just a relatively dumb object, and you’d have to draw a histogram like:

ROOT::TH1F hist(...);
ROOT::TGHist ghist( hist );
canvas.Draw( ghist );

In this case I would imagine ROOT::TCanvas having a function like:

void Draw( ROOT::TGHist& hist );

And I could also imagine TGHist having a constructor like:

template< class T >
TGHist( const ROOT::THist< T >& hist );

(Didn’t consider ownership issues too much here. Those should make things a bit more complicated of course.)

I’m of course really just throwing ideas around here. But I think it would be a much more natural behaviour for the “base” histogram type to behave just as a dumb object that has no “graphical behaviour”, and we would have to do something else in addition on top of it to deal with graphics.

But in the end this is not something that I’d go to war over. I would find it nicer if (base) histograms could be easily copied/moved/etc., but I’m not convinced that I know what would be the best UI to all of this.

Cheers,
Attila

This topic was automatically closed 14 days after the last reply. New replies are no longer allowed.

Fixed in master! Thanks for bringing this up!