shape operator[] and index() members

[psiha]

hi @jfalcou, finally back to trying to integrate kiwaku ;)
missing some interfaces, would it be possible to add:

• the ability to access values of individual shape dimensions with constexpr operator[] instead of/in addition to with the get<>() template member function - so that it works for runtime/dynamic and static arguments

• add an index() or offset() member function that returns a linear/flattened index or offset for a given set of coordinates, i.e. generalized equivalent of
  `std::uint32_t Dimensions::index( shape_t const dim0, shape_t const dim1, shape_t const dim2, shape_t const dim3 ) const noexcept
  {
  BOOST_ASSERT( dim0 < (*this)[ 0 ] );
  BOOST_ASSERT( dim1 < (*this)[ 1 ] );
  BOOST_ASSERT( dim2 < (*this)[ 2 ] );
  BOOST_ASSERT( dim3 < (*this)[ 3 ] );

  std::uint32_t const result( ( ( dim0 * (*this)[ 1 ] + dim1 ) * (*this)[ 2 ] + dim2 ) * (*this)[ 3 ] + dim3 );
  BOOST_ASSERT( result < count() );
  return result;
  }`

• also 🙈 maybe behind a 'EIGEN_INTEROP' define
  // implicit conversion to Eigen Tensor dimensions w/o Eigen includes
  constexpr operator std::array< int, 4 >() const noexcept;

------------ [EDIT JFALCOU]
Other element has been put in their own issues:

• Implements linear_index() -> coordinates conversion #33
• Generalize numel() that counts the volume of a slice #34
• Better constraints checking on shape #35

[jfalcou]

The first point is complicated. The shape interface is meant to have a tuple cause we can't have people write silly things like:

auto s = of_size(4_c, 8);
s[0] = 7; 

We can maybe have a read-only operator[] but mutable [] is a bit meh.

The second point, I don't get what you're computing, can you clarify? Is it the computation of a linear index from an nD position and a shape ?

[jfalcou]

Also note we're still hammering out the API, so can't really make any binding promise before v0.1

[psiha]

yes, read-only is perfectly fine! :)

second point - yes that's what i need

[jfalcou]

Ok for 1.
For 2. We re going to need some position class to do just that so the index(...) is coming.
If you need it, we will add it soon.

[psiha]

yes, i need it, soon ;D
but you don't 'need' a position class - you can make index a variadic template and/or accept an initializer list and/or an array...

[jfalcou]

Sorry typing from phone. I mean we need such a function for our position thing. We can add the other variadic interface too.

[psiha]

also nice to have would be the inverse of index() -> coordinates_from_index()

[jfalcou]

Smell like modulos :p

[psiha]

oh yes...modulos...more powerful than qovid (loss of smell) :D

[psiha]

• also a generalized numel() that counts the volume of a slice - taking start_axis and end_axis parameters

[psiha]

• also 🙈 maybe behind a 'EIGEN_INTEROP' define
  // implicit conversion to Eigen Tensor dimensions w/o Eigen includes
  constexpr operator std::array< int, 4 >() const noexcept;

[jfalcou]

also a generalized numel() that counts the volume of a slice - taking start_axis and end_axis parameters

This will be part of the slicing interface.

[jfalcou]

also 🙈 maybe behind a 'EIGEN_INTEROP' define // implicit conversion to Eigen Tensor dimensions w/o Eigen includes constexpr operator std::array< int, 4 >() const noexcept;

Not a fan of implicit conversion to low semantic type.
A as_array member is doable.

[psiha]

also 🙈 maybe behind a 'EIGEN_INTEROP' define // implicit conversion to Eigen Tensor dimensions w/o Eigen includes constexpr operator std::array< int, 4 >() const noexcept;

Not a fan of implicit conversion to low semantic type. A as_array member is doable.

not a fan of 'explicit interop' ;P
oh well minor issue now...

[psiha]

Also something more exotic like fits_static_constraints - a function that compares a 'less static' (dynamic_shape) with a 'more static' (reference_shape) shape by comparing/verifying that corresponding dimensions from dynamic_shape are equal to the corresponding statically defined/fixed dimensions from reference_shape...
for example
dynamic_shape = extent()()()()
reference_shape = extent()[100]200

bool fits_static_constraints() { return ( dynamic_shape[1] == reference_shape[1] ) && ( dynamic_shape[2] == reference_shape[2] ); }

This would be used in __builtin_assume() statements to tell the optimizer what it can assume about a given dynamic shape. For this to work (without compiler warnings about 'lost sideeffects') the function has to be marked with attribute(( const )) (or attribute(( pure )) for member functions, due to the access of this)...In general it would be helpful if you could make it a practice to slap those attributes on all appropriate functions (at least constexpr ones should be no brainers)..

[jfalcou]

This can be done by extending the contains and strictly contains member. Wonder if slapping the attribute on the == operator is enough.

[psiha]

operator== would compare all the dimensions while i want only the static ones used to verify the corresponding dynamic ones...
(regardless of that, a member operator== is a classic candidate for the pure attribute)

[psiha]

as an aside - pure and const attributes in general help the optimizer with things like subexpr elimination - for example it should be smart enough to see that shape::nbdims() can be hoisted outside a loop if the shape isn't modified but explicitly marking it so ensures it and speeds up that compilation pass (i.e. it can skip this analysis)

[psiha]

hm - speaking of read-only operator[] - how do you currently change a shape (i.e. the value/size of one of the dimensions)?

[jfalcou]

you currently dont. I am looking at how to make that proper.

Longer answer with a proper keyboard:

• is changing one piece of shape out context meaningful or should we have modifier functions/operators
• what to do when Bobby Tables try to change a fixed elements ? assert ?

[psiha]

• is changing one piece of shape out context meaningful or should we have modifier functions/operators

well it is certainly something 'people are used to doing' with other frameworks - but i can try and see how far can i get trying to make our code not rely on that (treating the entire shape as a single value that has to be changed when you want a resize)

• what to do when Bobby Tables try to change a fixed elements ? assert ?

well as always - not having the ability to do it in the first place (function removed by concepts) is the best, static assert is next and runtime assert the fallback solution..
(and throwing an exception if being compiled by committee members )

[jfalcou]

1. Yes I know but I wanted to see how far we can go before bowing to peer pressure. If it is required for usability, let's do it.

2. well concept removal for get is doable, I was thinking of the runtime [].

[psiha]

hm - back to index() - do i see correctly that the stride class provides that - so converting a shape to stride and calling index would get me what i want?

[jfalcou]

yup, and to convert a shape to a stride, use shape::as_stride()
I still think the index() thing is warranted.

[psiha]

ok thanks, any documentation/video on explaining the stride thing - how it differs from/constructs from/complements shape?

[jfalcou]

I think I explain it there:

https://youtu.be/IwBG_JjfcIA?list=PLyvPkeRjsYco_sUGIupgEu65igFNVu72k&t=2234

[jfalcou]

Note that the strides also simplify extraction of subviews and storage order handling
https://youtu.be/IwBG_JjfcIA?list=PLyvPkeRjsYco_sUGIupgEu65igFNVu72k&t=5157

[psiha]

I still think the index() thing is warranted.

yes - including the position class - or using a shape instance as a position

I think I explain it there:

thanks joel!

Note that the strides also simplify extraction of subviews and storage order handling

yes i saw that, it's great - however is it finished/operational? saw some commented-out code related to slicing..?

[jfalcou]

We are going to revamp the slicer API. the PoC worked so it's a matter of clean up. It is also related to the "waht is the shape when I took this shape and slice it ?" I think having slicing API on everything solves the thing properly.

Number of element in the slice of smade of all the rows and 1 over 2 columns ? s(all, by(2)).numel()

[jfalcou]

shape::as_array in #27

[jfalcou]

shape::operator[] in #28

[jfalcou]

Found a way to make get and operator[] works for mutating values when possible and to assert/static_assert if not.
This should fix the "how do I change shape" scenario

[psiha]

Found a way to make get and operator[] works for mutating values when possible and to assert/static_assert if not. This should fix the "how do I change shape" scenario

great, however this (mutable access) is not something you've added yet (as it does not work yet)?

[jfalcou]

The PR is running atm

[jfalcou]

#29

[jfalcou]

now it's merged ;)

[jfalcou]

linear_index is landing in #31
So any comments are welcome. It includes API for a variadic pack, tuple, and range of indexes.
When base_index and layout order land later, I'll amend it to support those.

[jfalcou]

Closing this for its initial purpose.
Remaining features moved to specific issues.