Re: [multi] Formal Review Begins
Hi Peter, thank you for your review Message: 3
Date: Mon, 9 Mar 2026 13:39:02 -0700 From: Peter Turcan <peterturcan@cppalliance.org> Subject: [boost] Re: [multi] Formal Review Begins To: "Boost developers' mailing list" <boost@lists.boost.org> Message-ID: <CABnUvfJMsz9AEa6epdBGC0= S4fEMcum5wbathXc+-zrXG5rqZw@mail.gmail.com> Content-Type: text/plain; charset="UTF-8"
Overall, the library should be usable with the current documentation, though there were quite a few issues that could be considered to improve it. Did largely like the tone of the text, the short and frequent examples, the correct use of formatting for code syntax, and the overall structure is about right. I appreciate the effort that has been put into this library so far.
Thank you for the appreciation.
1. Given that standard C++ already supports multi-dimensional arrays, I think it could be clearer in the Introduction what this library brings to the developer that is missing from the Standard. I would make the differences explicit - what features the Standard does not support. Perhaps this is just a wording change - from "Features of this library that aim to facilitate the manipulation of multidimensional arrays include:" to "Features of this library, that are not present in the Standard, that aim to facilitate the manipulation of multidimensional arrays include:". Though be careful to add further qualification to the features if any are present, in whole or in part, in the Standard. Is the Standard known for making "unnecessary copies" for example, if so, articulate this issue. Same deeper explanation for your other features.
This suggestion is accepted, I put these two sentences: "The following features of this library are not present in the Standard or generally combined in other libraries:" "For example, `std::mdspan` does not provide iterators, or generic interoperability with the rest of the STL." I also put a more specific things about mdspan in the Introduction.
2. In the Introduction again, I would have liked some more discussion on use cases. Multi-dimensional arrays are often used in real-time simulations for example, to provide physical data super-quickly without the need for equations or other computationally expensive approach. If there are other compelling examples (AI, finance, astrophysics, etc.), great to be specific and list them out. *This does have the effect of drawing in developers working in those areas - particularly if you can link the unique features of the library mentioned above to those use cases.*
Suggestion accepted: "In many simulations, space is divided into a grid, and values are stored at each grid cell. For example, arrays can represent fields of physical values, such as temperature variations within a block of material based on spatial coordinates, or velocity fields in a fluid. In a game, the state of board can be represented by placing pieces in a lattice. Arrays can also store numerical matrices, vectors, and more generally, tensors, to perform linear algebra operations. They can represent other abstract data structures, like network, graph, or store data in multiple columns. Arrays are amenable to bulk processing in computing, and also to slicing, and reductions."
3. Nit: typo - "althoug":
fixed
4. In the Intro there is this statement: Multi is a header-only library and C++17 or later is required. Good info, but other requirements are scattered throughout the doc. Perhaps have a "Requirements" sub-heading and add OS/Compiler or other requirements? Later on in the doc there is: The code requires any modern C++ compiler (or CUDA compiler) with standard C++17 support; for reference,... All statements like this belong in the Requirements section, which itself should follow the Introduction.
I put the requirements section after the introduction
5. Nit. Odd, truncated sentence in the table section: Why is dimensionality static and the sizes are dynamic? *Sizes are fixed or *
Fixed to: "Why is the array's dimensionality hardcoded, while the array's sizes can be defined at runtime?"
6. Navigation. Scroll to the end of a page there is no navigation options - great to see " *next*" and "*previous*" page links. Currently a bit tedious to have to relocate my place in the TOC.
Good point. I think this is a good feature for Antora.
7. Almost no linking to outside sources. For example, all the following sentences could contain a link (as could many others): Testing the library requires *Boost.Core* (headers), The library works out-of-the-box in combination with the *Thrust *library. The library works out of the box with Eric Niebler’s *Range-v3 *library, *CUDA *is a dialect of C++ that allows writing pieces of code for GPU execution, or with the standard *MDSpan* *proposal *std::mdspan. In the *next sectio*n instead we will see an example of arrays owned by the library. [link to next section where this example is]
I added links on most places, at least at the start of the corresponding sections.
8. Like the links to Compiler Explorer, but not the linking word "*online*" or "*live*" - doesn't say what the link is to - "Compiler Explorer" or whatever the title of the destination is would be much clearer.
I us the more explicit label "open with Compiler Explorer"
9. Nits. Remove unnecessary (and slightly condescending) words - *simple *or *a simple* adds no meaning (same for the word "very"), two examples: Once installed, other CMake projects (targets) can depend on Multi by adding *a* *simple *add_subdirectory ... change this to: Once installed, other CMake projects (targets) can depend on Multi by adding add_subdirectory ... Same for the following, and any other use of simply or very: —something manual indexing *simply *cannot do cleanly at scale.
I use the more descriptive phrase: "... by adding a *single line* `add_subdirectory(my_multi_path)`" I removed all instances of *simply* and *very*. Reduced the number of *simple* occurrences.
10. Only add brackets where really necessary, they are an obstacle to smooth reading, such as here: (Although most of the examples use numeric elements for conciseness, the library is designed to hold general types (e.g. non-numeric, non-trivial types, like std::string, other containers or, in general, user-defined value-types.) A (mutable) array can be assigned (The Cereal XML and Boost XML output would have a similar structure.) (de)serializing [spell it out "when serializing or deserializing a ..."] - and many more examples.
All these suggestions accepted and implemented. I will not use parenthesis clauses in the future and remove more when I see them.
11. Perhaps "Getting Started" is a friendlier title than *Primer (basic usage)* and "Tutorials" better than *Tutorial (advanced usage)*. However, the tutorials are not really tutorials unless they contain numerical steps that a student can follow. As it is - *Advanced Usage Examples*, or a similar title, is more accurate.
I copied it from other Boost documentations, but the suggestion is accepted and implemented.
12. Nit. Typo: Because this array is not *controled* by the library, this type of object is called an array-reference. Correct spelling is controlled.
fixed
13. Nit. Typo: why the colon?
Note that this is also different from creating a named *reference:*
(probably there was code after that)
fixed
14. Style nit. Make reference material impersonal - not "In my experience, however, the following" more "Usage patterns can produce .....". Library documentation is formal and not a blog post, so "opinions" are mostly inappropriate - expressing factually with conditions is more appropriate.
fixed
15. Typo - sentence starting with lower case: *which* uses the default Thrust device backend (
fixed
16. Typo - upper case C needed: Why is *c++*17 chosen as the minimum?
fixed
17. Not crazy about punctuation in headings, for example: *{fmt}* - would be better as "Formatting" *Copy, and assigment (, and aliasing)* is ugly, "Copy, Assignment and Aliasing" works better
fixed
Perhaps use Title case for headings - capitalize nouns, verbs, adjectives, not articles, prepositions or conjunctions. Avoid punctuation if at all possible.
fixed
18. Not sure about the following example code: Compile time evaluation constexpr auto trace() { multi::array<int, 2> arr = {{1, 2, 3}, {4, 5, 6}, {7, 8, 9}}; arr[2][2] = 10; return std::accumulate(arr.diagonal().begin(), arr.diagonal().end()); }
static_assert( trace() == 4 + 2 + 10 );
If arr[2][2] is 10, shouldn't the assert be "1 + 5 + 10", or have I misunderstood "diagonal"?
you are right the example was wrong, I corrected it.
19. Reference - I agree with earlier comments that the Reference should be more complete. Each function/method should have its own page (or section) to include a brief sentence on its purpose, then syntax, parameters, return values, errors and exceptions, remarks for a fuller description if necessary, and for bonus points an example, or link to an example, of its use.
Yes, the reference section needs a lot of work. I am trying to find the right tool for this, Mr.Docs. For example. I am looking at the example from Capy. A peculiarity of this library is that many of the input and output parameters are conceptual, not concrete types. This in turn calls for a Reference for the concepts.
20. Appendix - How to's is not appendix material - belongs after or part of tutorials (perhaps renamed to "Advanced Use Cases" or similar).
It was a request in another review, I renamed it as FAQ.
Currently my feelings are that this library should have a CONDITIONAL ACCEPTANCE that the features added are a significant enough improvement over the current Standard, and that those improvements are clearly articulated in the Introduction section of the docs. And some work on the Headings, Reference and Navigation issues would help a lot.
Hope this helps.
Yes, it helps a lot. Reference requires work, but I hope I solved most of the other problems. Let me know if the Intro is more convincing now. Thank you, Alfredo
participants (1)
-
Alfredo Correa