Re: [boost] Formal Review of Proposed Boost.Histogram Library Starts TODAY

Subject: Re: [boost] Formal Review of Proposed Boost.Histogram Library Starts TODAY
From: a.hagen-zanker_at_[hidden]
Date: 2018-09-26 13:07:54

> you don't need to define my_any yourself, if you use the make_*_histogram factories, as shown in the user guide. Those work automatically with custom types.

I took you second example as starting point and did whatever necessary to make it work with my own axis. That example starts by putting axes into a vector and then uses the make_dynamic_histogram. I need my_any as value_type for this vector.

***********
using my_any =
    bh::axis::any<bh::axis::regular<>, bh::axis::category<std::string>, my_axis>;

int main() {

  std::vector<my_any> axes;

  axes.emplace_back(bh::axis::category<std::string>({"red", "blue"}));
  axes.emplace_back(bh::axis::regular<>(5, -5, 5, "x"));
  axes.emplace_back(my_axis{});

  auto h = bh::make_dynamic_histogram(axes.begin(), axes.end());
************

> I am open to renaming axis::any, but isn't axis::variant equally confusing? I avoided the name "variant", because axis::any does more than a normal variant, it implements the common interface of the builtin axis types to provide some convenient polymorphism. axis::any cannot represent all aspects of all axes, but it works for simple cases at least.

I don't know what you mean by "cannot". If you define a consistent Axis concept, you can wrap this in a type-erased axis::any. If you chose not to do this, then axis::variant is a less confusing name to me.  

>> "value()" seems to be a misnomer, since it is used to print a label for the bin (in your second example: getting_started_listing_02.cpp line 41). To me it is not logical for a bin to have a value() member function, since a bin really is a domain of values.

> That depends on the axis. axis::category has bins which represent a single value, not a continuum.

A single value, a set of values and a continuum of values are all domains of values. A bin logically is a domain of values. 

> That's the example you are referring to. Same for axis::integer. The other axes types use a different kind of bin type that represents an interval. That's why there are two helper templates interval_view and value_view. Use the former for axis types whose bins represent an interval and the latter for axis types whose bins represent a single item.

I don't agree, a bin always is a domain of values and there are more options than interval and value. For instance,  I could chose to have bins for odd and even integers. I think you need an interface for the generic case: a bin is a domain.

>> operator int() seems inappropriate because why would a bin need to 
>> know its index? That should logically be the responsibility of the 
>> axis

> I explained the benefit in an answer to Steven.

But, I don't think that convenience is sufficient reason. It is not logical and brings about further questions. A histogram is a relatively simple mathematical construct, it must be possible to support it with concepts that are rigorously logical, not just convenient. 

>> 3. I think this was covered before, but the inheritance from labeled_base is awkward. I am not just required to inherit from it, but also construct it in the appropriate form.
>> 
>> I ended up constructing the base class with:
>> 
>> my_axis() : labeled_base(2, bh::axis::uoflow_type::off, "positive vs. 
>> negative", std::allocator<char>{}) { ...
>> 
>> It seems too onerous to require my own axis to know the number of bins on construction. What if I want to add bins one at a time? What if my number of bins is based on some function?
>> I also really don't like having to bother with std::allocator<char>; I normally stay away from that kind of complexity. Why should using histograms force me to start thinking about allocators?

>The allocator argument can get a default so you don't have to see it. To be honest, I didn't think someone would want to make axis from scratch. Usually, you want to bend behaviour of an existing axis a bit to achieve what you want, and that is much simpler, see the example in the user guide.

Even though in practice that is what I might do, I am not sure if it is really simpler. I would just be less aware of what I was doing. 

>> 4. Using iterator_over might not be efficient
>> 
>> If my axis stores bin_types in a std::vector (which seems reasonable), why can I then not just use the iterator of std::vector? You could achieve that by leaving it up to the user to implement the iterators, and offer iterator_over as the default..
>> 
>> I still think histogram should be accepted, but in my opinion there remains quite some scope to tidy up the concepts to make working with histograms as simple as possible.

> I am thankful for your input on the advanced usage of the library, and of course that should be as simple as possible and it needs better documentation, but you cannot expect it to be as simple as the basic usage. A good library should make simple things simple and complicated things possible.

Hmm, in my opinion defining your own axis should be a simple thing in a histogram library. Moreover, I think a mathematical library should map mathematical concepts to programming concepts as closely as possible.