Re: [multi] Formal Review Begins
Alfredo Correa via Boost <[email protected]>
| Newsgroups | gmane.comp.lib.boost.devel |
|---|---|
| Message-ID | <CALy_jFoqrPStP6J1+ihuXK2v=j4gEsmvhEvFCRcOP17rzNzUNw@mail.gmail.com> |
Dear Rainer, its Alfredo again
I realized that the example that you brough can be written more compactly
as (no need for std::get or std::apply)
```
multi::array<int, 2> a({3, 2});
for( auto [i, j] : a.extensions().elements() ) {
a[i][j] = i + 10*j;
}
```
https://godbolt.org/z/4EEsode6b
For reference your original challenge (usinb your library was)
```
multi_array<int, 2> a({5, 2});
for (auto index: a.get_bounds()) {
a[index] = get<0>(index) + get<1>(index) * 10;
}
```
I hope this address your observations of accessing "array-of-array" logic
vs. "flat elements" logic, Multi supports both seamlessly.
Thank you again,
Alfredo
On Mon, Mar 9, 2026 at 12:30 PM Alfredo Correa <[email protected]>
wrote:
> Dear Rainer,
>
> Thank you for the two messages and for taking the time to review the
> library in detail.
>
> Your first message compares this library to your own library that I don't
> know about.
> While I don't want this response to be become a comparison between two
> libraries, I will indicate the points where you misrepresent the Multi
> library.
>
> I take the criticism of the first message, regarding flat iteration vs.
> multdimensional iterator seriously because this is exactly one of the main
> things the library is trying to solve.
>
> The second message is a formal review, but maybe after reading this first
> answer you can reconsider the library under a different light and it will
> be easier to respond to the second message (formal review)
>
> This email answer both messages together:
>
>
> On Sat, Mar 7, 2026 at 1:44 PM <[email protected]> wrote:
>
>>
>> Message: 2
>> Date: Sat, 7 Mar 2026 10:57:01 +0100
>> From: Rainer Deyke <[email protected]>
>> Subject: [boost] Re: [multi] Formal Review Begins
>> To: [email protected]
>> Message-ID: <[email protected]>
>>
>> The proposed library uses an array-of-arrays model.
>
>
> Multi supports both models array-of-array and flattened view this is
> prominently mentioned in the documentation, in the Iteration section (
> https://correaa.github.io/boost-multi/multi/tutorial.html#tutorial_iteration
> )
>
>
>> By contrast, my own
>> library uses an array-indexed-by-vector model. This makes iteration a
>> lot more convenient with my type. I do a lot of iteration in my own
>> code, so this is kind of a big deal. Compare:
>>
>> My multi_array type:
>>
>> multi_array<int, 2> a({5, 2});
>> for (auto &element: a) {
>> element = 5;
>> }
>> for (auto index: a.get_bounds()) {
>> a[index] = get<0>(index) + get<1>(index) * 10;
>> }
>>
>> The proposed library:
>>
>> multi::array<int, 2> a({5, 2});
>> for (auto &row: a) {
>> for (auto &element: row) {
>> element = 5;
>> }
>> }
>> auto [rows, columns] = a.extensions();
>> for (auto row_index: rows) {
>> for (auto column_index: columns) {
>> a[row_index][column_index] = row_index + column_index * 10;
>> }
>> }
>>
>> Aside from more compact code, one advantage of this kind of flattened
>> access is that it makes it possible to write non-recursive generic
>> functions that work on arrays with different dimensionality. It should
>> be possible to add this kind of flat iteration to the proposed library
>> without breaking any existing functionality.
>>
>
> As I said, the library already supports the flat iteration and I agree
> that this is necessary, it was one of the main motivations of the library.
> The documentation uses the same vocabulary even.
>
> Your two cases can be written in this way with the library:
>
> multi::array<int, 2> a({5, 2});
> for(auto& element : a.elements()) {
> element = 5;
> }
>
> for(auto indices : a.extensions().elements() ) {
> apply(a, indices) = get<0>(indices) + 10*get<1>(indices);
> }
>
> https://godbolt.org/z/WeWKTnrb5
>
> This adds to Andrzej answer to this point in previous email where he
> correctly indicated that the first case can do with a one liner:
> `std::ranges::fill(a.elements(), 5);`.
>
>
>> The proposed library makes extensive use of array views. This is useful
>> both for performance and functionality, but it's also dangerous because
>> it can lead to unintentional data aliasing and dangling pointers. My
>> own approach is to just have the single owning multi_array type and no
>> array views at all. This means a few extra unnecessary copies of data,
>> but for my usage the performance penalty is completely acceptable.
>>
>
> Not arguing with that both approaches are acceptable.
> Multi supports both, if you don't want to make views you can use them as
> temporaries and never assign them names or return them from functions.
> Multi will be pretty happy to let you make copies all around, it has
> mechanisms for that too.
>
>
>>
>> The proposed library provides just one function for changing the extents
>> of an existing array: reextent. My type has member functions for
>> inserting and removing
>> rows/columns/planes/whatever-the-n-dimensional-equivalent-is, and I use
>> them all the time. This is no big deal, since this functionality can be
>> provided by external free functions.
>>
>
> The main reason not to provide insertion is that the library does not
> amortize allocations.
> New arrays can always be made it insert rows and columns.
>
>
>> ------------------------------
>>
>> Message: 3
>> Date: Sat, 7 Mar 2026 15:47:27 +0100
>> From: Rainer Deyke <[email protected]>
>> Subject: [boost] Re: [multi] Formal Review Begins
>> To: [email protected]
>> Message-ID: <[email protected]>
>> Content-Type: text/plain; charset=UTF-8; format=flowed
>>
>> This is my formal review of Boost.Multi.
>>
>> On 3/5/26 14:26, Matt Borland via Boost wrote:
>> > - What is your evaluation of the design?
>>
>> While the array-of-arrays view of a multidimensional array can
>> (occasionally) be useful, I find the lack of direct element addressing
>> using a tuple/array/vector type problematic.
>>
>
> The library provides both array-of-array addressing and element address as
> illustrated in your previous message. Both of these codes are acceptable:
>
> multi::array<int, 2> a({5, 2});
> for(auto& element : a.elements()) {
> element = 5;
> }
>
> for (auto &row: a) {
> for (auto &element: row) {
> element = 5;
> }
> }
>
> https://godbolt.org/z/WeWKTnrb5
>
>
>> The rotated/unrotated/transposed member functions are specific cases of
>> the more general concept of changing the order of the dimensions. A
>> simple direct function should be added.
>>
>> The use of "rotated" to switch the order of dimensions is unfortunate
>> because it conflicts with the meaning of rotating an image, i.e. turning
>> this:
>> {{1, 2, 3},
>> {4, 5, 6},
>> {7, 8, 9}}
>> ...into this:
>> {{7, 4, 1},
>> {8, 5, 2},
>> {9, 6, 3}}
>> Instead, it turns it into this:
>> {{1, 4, 7},
>> {2, 5, 8},
>> {3, 6, 9}}
>> ...which, geometrically seen, is flipping operation over the diagonal,
>> not a rotation.
>>
>
> The name is based on the "rotation of indices",
> a(i, j, k) turns into a(j, k, i), etc.
>
> Take into account that the names must apply in the multidimensional
> context. There is a collision of interpretation in the 2D case here.
>
>
>>
>> "taked" is not a word in the English language. Based on the context, I
>> think the correct word is "taken".
>>
>
> Yes, I think both names should be provided.
> The idea was that all views are generated with '...ed' functions, to
> stress that it is not a verb,
> and in this case by using a colloquial conjugation of the verb `take`:
> https://en.wiktionary.org/wiki/taked
>
>
>> The use of unary operators + and ~ on arrays conflicts with the
>> potential elementwise use of the same operators:
>>
>> using boost::multi::elementwise;
>> boost::multi_array<my_class, 2> a{2, 3};
>> auto b = -a; // Creates a lazily evaluated array that applies
>> // operator- to its elements.
>> auto c = +a; // Creates an eager copy of the array, without applying
>> // operator+ to its elements.
>>
>
> The encouraged use of unary operator+ helps create copies and the use of
> auto.
> multi::elemntwise::operator+ is binary. In any case, semantically both do
> the correct thing.
>
> I think this is the cost of blanket `using namespace ...` in the first
> place.
> Which I don't recommend.
> The design is a compromise.
>
>
>> The name "dynamic_array" implies that it is more dynamic than plain
>> "array", but the opposite is true. This class should be renamed,
>> possibly to "fixed_array" or "static_array".
>>
>
> It was called `static_array` until recently until I realized that the name
> was problematic.
> It is called `dynamic_array` because it uses dynamic memory and runtime
> sizes.
> This is the agreed upon meaning in the community.
>
> See also proposals for `dynarray` that is exactly the same idea, (an array
> in the heap without pointer invalidation).
> https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2013/n3810.pdf
>
>
>> The boost::multi::array::reextent function is not clearly defined.
>> Given this input:
>> {{1, 2},
>> {3, 4}}
>> what happens if I call reextent({3, 1}, 0)? Do I get this:
>> {{1, 2, 0}}
>> or do I get this:
>> {{1, 2, 3}}
>>
>
> You will obtain {{1, 2, 0}}, a default element will appear at a[0][2]
> because a[0][2] was not part of the original array.
>
> Your second option depends on some interpretation of the array as flat
> array of numbers.
>
>
>> I don't like the name "restriction" for a lazy array.
>>
>
> Restriction is a well defined mathematical concept, you restrict a
> function to a domain. The domain is the grid of valid indices.
> https://en.wikipedia.org/wiki/Restriction_(mathematics)
>
> The problem with calling it "lazy" is that the meaning is too vague. Many
> libraries provide "lazy" sequences, they mean different things in different
> contexts. F
>
>
>> I don't like the name "extensions" for what I would call "extents".
>>
>
> This has been brough up before, `extensions` will be renamed `extents`.
>
>
>> The factory function "restricted" takes its argument in the opposite
>> order from how the class "restriction" takes its template arguments. Why?
>>
>
> restriction (the class constructor) takes the arguments in the same
> arguments are the rest of the library, extents, (e.g. {5, 2}) and, as
> second argument, the the "source" of the elements, an allocator (for
> multi::array) or a pointer (for multi::array_ref), and in the case of
> multi::restriction the second argument is the function that generates the
> values.
>
> restricted (the factory function) reads a restricted(f, domain), f is
> restricted to the domain.
>
> There are no more reasons than this.
> It seems that using the factory function is always more convenient.
>
>
>> The constructors of boost::multi::dynamic_array and boost::multi::array,
>> as well as the function boost::multi::array::reextent, require copy
>> construction. It would be more flexible to allow emplacement from any
>> set of variables that can be used to construct an element, similar to
>> std::vector::emplace_back.
>>
>
> I thought about this, but it is a false optimization.
> Basically the idea is that if you want to generate nxm copies of an
> elements, it is better to first make a master copy at the call site and the
> copy this elements. That is how std::vector constructor works. Optimizing
> for the 1-element arrays is not useful in the context.
> This is fundamentally different from ::emplace_back.
>
> Yes, you could do pretty powerful stuff this way (like having a
> constructor that populates the array with variable values, but using an
> intermediate class).
> But you can do this better with restrictions, and second, variadic
> templates in a constructor can mess up think in a nasty way.
> multi::array({5, 2}, arg1, arg2, arg3); (to construct elements T{arg1,
> arg2, arg3}).
>
> Having said that I can add tagged constructor,
> `multi::array(std::in_place, {5, 2}, arg1, arg2, arg3);` if you think it is
> something that you need.
>
>
>
>> > - What is your evaluation of the implementation?
>>
>> I only took a cursory look at it, mostly to make up for deficiencies in
>> the documentation.
>>
>> > - What is your evaluation of the documentation?
>>
>> It's pretty bad.
>>
>> On a technical level, the web page doesn't accept arrow keys or page
>> up/down for scrolling, so I have to rely use my mouse for scrolling.
>>
>
> I am able to navigate, scroll up and down with arrow keys, page up/down,
> and space.
> I am using a state of the art (I am told), documentation format (Antora).
>
>
>> Also on a technical level, the right side of some of the tables is cut
>> off on my web browser even though there is blank space to the right of
>> the table. I am forced to rely on horizontal scrolling to see the whole
>> table, and there isn't even a horizontal scroll bar.
>>
>
> Except in the reference section, I have no problems rendering the tables
> on web browsers, all fit nicely in my browser window (Safari on Mac,
> Firefox on Linux).
> Can you send a screenshot of what you see so Matt Borland and other from
> Boost can help me make them fit in your window.
>
> I see something similar to what you describe in the reference section, I
> will ask for help to the authors of Antora to see if they can fix it. (I
> think this because the table contains code)
>
> It will be fixed soon.
>
> UPDATE: Matt opened this issue:
> https://github.com/boostorg/boostlook/issues/174
>
>
>>
>> The tutorial recommends using "auto &&" or "auto const &" for holding
>> subarrays instead of "auto". I disagree. A subarray is not a
>> reference, no matter how much it tries to act like one, and holding it
>> by reference is a good way to get a dangling reference. I am fully
>> aware that local variables of reference can extend the lifetime of a
>> temporary, but I consider it messy to rely on this unless absolutely
>> necessary.
>>
>
> The use of the library doesn't depend on you following this advice.
> subarrays are not language-references and you are right.
> You can use plain `auto` if you want to follow a more canonical convention.
> You don't need to rely on this, and the library itself doesn't rely on it.
>
>
>> The reference for functions does not include the full function
>> prototype, with return type and argument types.
>>
>
> This is being fixed.
> The main problem is that function returns depend on lots of template
> parameters so a more conceptual explanation is being preferred.
> Having said that this is going to improve.
>
>
>> The boost/multi/elementwise.hpp header is not documented at all in the
>> reference section.
>>
>
> It is documented here:
> https://correaa.github.io/boost-multi/multi/tutorial.html#tutorial_elementwise
>
>
>>
>> The documentation for (I assume)
>> boost::multi::subarray::const_element_ref just calls it element_ref.
>>
>
> For restrictions? not necessarily, depends on the actual element returned,
> but what is your question?
>
>
>> boost::multi::subarray::index_range and
>> boost::multi::subarray::extension_type are only described in very vague
>> terms. What are the members of these types?
>>
>
> index_range is whatever type you put in the parenthesis notation to
> indicate a range of elements
>
> A( {2, 4} , 5 ); // is the array {A(2 , 5 ), A(3, 5} }
>
> {2, 4} is converted to index_range.
>
> Extension type is whatever is returned by the .extension() function (this
> is like .size() but returns a range)
>
> multi::array<int, 2> a({5, 2});
> a.extension(). is the range 0, 1, 2, 3, 4. It is of type
> subarray::extension_type.
>
> It helps writing loops
>
> for(auto idx : a.extension()) {
> ... a[idx] ...
> }
>
>
>
>> The return types of boost::multi::subarray::elements and
>> boost::multi::array_ref::elements should be documented.
>>
>
> Well the type is a complicated type, that for the most part you don't need
> to know about.
>
> multi::elements_range_t<int*, multi::layout_t<2, long int> >
>
> It depends on other template parameters. It suffices to know that is a
> random access range.
>
>
>> The reference for boost::multi::dynamic_array::elements says that it is
>> overridden from its base type. This is not true, it inherits from
>> boost::multi::array_ref::elements (which does override
>> boost::multi::subarray::elements).
>>
>
> You are correct. It was a copy paste problem.
>
> It is fixed now.
>
>
>> The references for dynamic_array and array have a bunch of references to
>> array_ref and dynamic_array respectively for functionality that is
>> ultimately inherited from subarray. It would be better to point these
>> references directly to where the functionality is defined.
>>
>
> I was discuraged from referring the documentation of the base class so i
> started doing copy paste.
>
>
>>
>> boost::multi::restriction has a member function called "home" which
>> "returns an indexable cursor". I don't know what this cursor is, what
>> it does, or what members it has.
>>
>
> Cursors are described in other sections.
> I will add a section on the tutorial, its uses are pretty advances.
> For arrays, It is for context in which you can't use pass-by-reference.
>
>
>> boost::multi::inplace_array is mentioned once but never documented.
>>
>
> I have to add a reference section for inplace_arrays.
>
>
>> "operator⇐" should be "operator<=".
>>
>
> Fixed
>
>
>> > - What is your evaluation of the potential usefulness
>> > of the library? Do you already use it in industry?
>>
>> I think a good multidimensional array type is very useful, and I use my
>> own multidimensional array type all the time. The question is if
>> boost::multi::array qualifies as a good multidimensional array type.
>>
>
> It seems that the main technical criticism was regarding features that you
> can opt-in, like views, but that you are not forced to use.
>
> Direct element access is something that you consider very useful and a
> deal-breaker for any multidimensional library.
>
> I hope I convinced you that element address is a first class citizen in
> the library and it was a priority since day one.
>
>
>>
>> > - Did you try to use the library? With which compiler(s)? Did
>> > you have any problems?
>>
>> No.
>>
>> > - How much effort did you put into your evaluation?
>> > A glance? A quick reading? In-depth study?
>>
>> I spent most of the day on it.
>>
>
> Thank you very much for the effort.
>
>
>> > - Are you knowledgeable about the problem domain?
>>
>> I've written my own multidimensional array class and I've used it
>> extensively.
>>
>
> I welcome comparisons in features and ergonomics.
> Your demonstrations helped me think how to express this with my library.
>
>
>> > Ensure to explicitly include with your review: ACCEPT, REJECT, or
>> CONDITIONAL ACCEPT (with acceptance conditions).
>>
>> I would love to have a good multidimensional array library in Boost.
>> Unfortunately, Boost.Multi is not such a library in its present state.
>> At the very least, I would need the following changes before I can
>> accept the library:
>>
>> - The documentation needs to be completed.
>>
>
> I am working in improving the documentation.
>
>
>
>> - I want to either be able to index the array using a
>> tuple/array/vector type, or a damn good rationale or why this isn't
>> possible.
>>
>
> You can do this already.
>
>>
>> Unfortunately the problems with the documentation are so severe that I
>> feel that another review is needed after the documentation is completed
>> before the library can be accepted. I therefore vote to REJECT this
>> library at the current time in its current state.
>>
>
> I will work intensively in the documentation points you raised, especially
> if that opens an opportunity for a conditional acceptance.
>
> Thank you very much,
> Alfredo
>
_______________________________________________
Boost mailing list -- [email protected]
To unsubscribe send an email to [email protected]
https://lists.boost.org/mailman3/lists/boost.lists.boost.org/
Archived at: https://lists.boost.org/archives/list/[email protected]/message/Q3Y7ALH4XUVESSVV53SLSZLPZ6EJOLAY/