Re: [gsoc2020] Answer to histogram design questions from Slack

From: Pranam Lashkari (plashkari628_at_[hidden])
Date: 2020-06-07 05:54:32

On Fri, Jun 5, 2020 at 12:52 PM Debabrata Mandal <
mandaldebabrata123_at_[hidden]> wrote:

> *Answer to the post above* : I had mentioned unordered_map due to the
> reason it employs hashing which is similar to the way Boost.Histogram maps
> keys to unique bins in a storage class. This will become particularly
> useful for signed images of any channel type. At the same time an
> unordered_map does not guarantee key value mapping to be present for every
> pixel value in a given range. So a sequence container like array or vector
> would be better in this case.
> The histogram class has a clear distinction from the kernel class, in the
> sense that the kernel needs a contiguous storage while the histogram does
> not.
>
Yes, in this case, it makes sense to use the unordered map and the problem
of not having all the key map value mapping can be solved with inheriting
and creating a new class from unordered_map and then overriding
`operator[]` and `at()` because all the non-existing keys would have
value zero and this may make things simpler. Correct me if I am missing
anything here.

Further external classes like Boost.Histogram might still need template
> specialisation to distinguish between the implementations.
> FYI, Boost.Histogram solves a similar problem by letting the user provide a
> custom storage class that models the histogram concept that it follows.
>

> Further, I believe it is not practical to provide the entire interface
> shipped by an already sophisticated histogram class like Boost.Histogram.
> The reason I mention this is because while Boost.Histogram might provide a
> special function which is available only if we use Boost.Histogram as our
> background storage, providing a similar function for std container like
> vector may prove inefficient which then leads to the problem of
> inconsistency. We need to make the end user accountable for the interface
> they desire which can be done only if we give control over the container.
>
For now let's not talk about what boost histogram does because, in my
opinion, we are an image processing library and not histogram so we are
good with having basic functionality.

What's left is what can be done for the std containers. We have *two*
> approaches in hand :-
> 1. Overload design - One of the drawbacks in picture is the usage syntax
> which might be difficult to use, but we cannot avoid that in the case of
> class approach either if we want future integration with external classes.
> Another drawback would be the difficulty in tracking which functions get
> called for a particular container type. This is a problem because when we
> add histogram utilities like mean, normalization etc. , they might need
> different implementations for different classes which is why it would be
> difficult to identify the right function unless they are properly commented
> and documented.
>
This is the approach you have used in the PR which I reviewed and after
looking at the prototype/early code I think this is not the way we should
ever choose it is less flexible and introducing even a small change in the
future would lease to changing many things and it will have high
maintenance cost in terms of time and code.

2. Class approach - To decide on a particular class type, means deciding
> against future extensions to the container type, which is not a plus point
> if someone comes up with an implementation of efficient Histogram class for
> Boost.GIL. To further support this argument I can provide the
> implementation design of OpenCV which separates the *conversion routine* of
> image to histogram from the *implementation *of the actual histogram class.
> Although they do not provide overloads for their container type, they have
> their own histogram class to rely on. Since GIL lacks this feature,
> wouldn't it be better to allow work on this in the future?
>
I am in favour of this as I have mentioned earlier. We always have to
compromise something there can not be a perfect design always

A third approach would be to provide a mix of classes for std containers
> and overload for external classes which will deal with the problems of both
> the above two points.
>
Can you show us a prototype to get a clearer idea?

So a clear way to me (which I had mentioned in the proposal as well) is to
> provide the conversion routine for now, independent of the container class
> type, but ofc this is my solo opinion and might not be the best. I wish to
> know what the opinions of the community are?
> Further details on the overload design are provided in the github comment
> <https://github.com/boostorg/gil/pull/499#issuecomment-637746390> .
>  I hope I could clearly express my views on the topic.
>

-- 
Thank you,
Pranam Lashkari