boostorg / histogram

Fast multi-dimensional generalized histogram with convenient interface for C++14
Boost Software License 1.0
316 stars 70 forks source link

check run-time version shrink, rebin, shrink_and_rebin when non-supported axis is there #179

Closed HDembinski closed 5 years ago

henryiii commented 5 years ago

I believe this could be done rather easily, by splitting the visitor on the axis into two separate functions, one that performs the action on a valid axis, and one that throws an error on an invalid axis.

HDembinski commented 5 years ago

This needs some TMP magic to detect the presence of the special constructor that enables the support for shrink, rebin and so on.

henryiii commented 5 years ago

Yes, that's what selects which function to instantiate in the visitor. I already have a rough draft that works, but have had issues with the other bugs that I've mentioned. I'm also using true_type and false_type, rather than a more elegant method, since there are several ways to do it and I'm not sure which is recommended for Boost::Histogram for this sort of selection.

HDembinski commented 5 years ago

Using tag types is fine.

HDembinski commented 5 years ago

static_if may also be handy

henryiii commented 5 years ago

Tag types require the caller to do the check. It's nicer if the caller doesn't have to know about it. (since it's a detail function it's not too bad).

HDembinski commented 5 years ago

I don't know whether we talk about the same thing, by tag type I mean an extra argument to function, to trigger the right overload. That can be std::true_type or std::false_type. In any case, I am looking into this. This seems to be a subtle problem. It could be related, however, to some other errors you experienced where you asked me whether some axis type is not copy constructible. There may be a bug in the axis::variant which is causing this.

henryiii commented 5 years ago

Yes, that's what I meant. The downside is the caller has to pass the flag type as an extra argument to the function, rather than just having it act on the types already present in the function signature. But it's simple and since it's private, is not a bad solution, just not as elegant as I would have liked. I'm currently setting up tag types to turn on/off weighted fills based on the storage.

HDembinski commented 5 years ago

There is no better solution than this if you need to do static dispatch to methods.

HDembinski commented 5 years ago

With static_if you can do it without adding private methods, but it has other drawbacks.

HDembinski commented 5 years ago

fixed on develop