FAQ initial revision #59
Conversation
| // A: Use `auto_allocator<>` as the allocator for your arrays. | ||
|
|
||
| // Define an allocator that has storage for up to 100 elements. | ||
| using StackAllocator = auto_allocator<int, 100>; |
There was a problem hiding this comment.
Oof, it took me a few minutes to wrap my head around this. std::allocator is stateless, but auto_allocator is not - it owns a buffer. I was trying to figure out where it's allocated - only to remember that it's a template argument - so the instance actually owns the allocator instance!
Not sure how to point this out (or if that's necessary. If you understand allocators then it's not a problem, and the documentation assumes you're pretty good at C++ :)).
There was a problem hiding this comment.
Yes, this is something that needs more documentation, in the array.h header actually. The docs on these allocators should explain they can't actually be used with STL containers, because they're stateful.
It's actually really annoying to me that STL allocators are stateful because they preclude exactly this use case, which is very powerful. Replicating absl::InlinedVector goes from just a simple allocator like this one to basically a total rewrite of std::vector :(
There was a problem hiding this comment.
It's actually even more annoying than I remembered: STL allocators have all this machinery that looks like it attempts to enable stateful allocators, like these flags such as propagate_on_container_assignment and select_on_container_copy_construction. But no STL I tested respects those flags, argh! This is issue #7
| // This can fail at runtime if the source shape is not compatible with the | ||
| // destination shape. | ||
| } | ||
|
|
There was a problem hiding this comment.
How about a few more FAQs on what shape::resolve() does, and what shapes or array_refs can be passed to functions with automatic conversion?
There was a problem hiding this comment.
Hmm, can you share a case in which you actually needed to care about resolve? I think it's not something that normally you should have to think about.
| @@ -2737,7 +2737,8 @@ const_array_ref<U, Shape> reinterpret(const array<T, Shape, Alloc>& a) { | |||
| * `new_shape`, with a base pointer offset `offset`. */ | |||
| template <class NewShape, class T, class OldShape> | |||
| NDARRAY_HOST_DEVICE array_ref<T, NewShape> reinterpret_shape( | |||
| const array_ref<T, OldShape>& a, const NewShape& new_shape, index_t offset = 0) { | |||
| const array_ref<T, OldShape>& a, NewShape new_shape, index_t offset = 0) { | |||
There was a problem hiding this comment.
Some places, array_ref and shape is passed by const ref, elsewhere, by value. What's the recommendation?
I think small things should always be passed by value in modern C++?
There was a problem hiding this comment.
I agree passing these by value is quite reasonable. That said, they aren't that small if the shape has a few dimensions (each dim has the same size as 3 pointers). I changed this one to be by value because the implementation needs to mutate it, so it would be making a copy anyways.
In this case it doesn't really matter, but for non-trivial types, this is a lot better (enables move construction if the caller wants that, instead of construct + copy).
|
Just a few nits - do you want to merge this? |
Co-authored-by: Jiawen (Kevin) Chen <[email protected]>
Here's the first few things I thought to put in an "FAQ". It's written as a test (like readme.cpp).