libmir / mir

Mir (backports): Sparse tensors, Hoffman
http://mir.libmir.org
Boost Software License 1.0
210 stars 20 forks source link

consistently accept input shape as a static array only #337

Closed timotheecour closed 8 years ago

timotheecour commented 8 years ago

I'd like mir to consistently accept input shape argument as a static array, never as separate indexes, kind of like numpy:

a=numpy.zeros((2,3))//ok
a=numpy.zeros(2,3)//bad

Currently in mir this all works:

  auto a0=iota(3*4).sliced(3,4); // proposal:deprecate
  auto a1=iota(3*4).sliced([3,4]);
  auto a2=iotaSlice(3,4); // proposal:deprecate
  auto a3=iotaSlice([3,4]);
  auto a4=slice!int([3,4]);
  auto a6=slice!int(3,4);// proposal:deprecate
  // a bit inconsistent: only static array form is allowed when passing an extra argument
  auto a7=slice([3,4],5); 

Several reasons for changing this:

9il commented 8 years ago

I am not against it, but need more opinions about this.

John-Colvin commented 8 years ago

I'm in favour of this, because users can always use [myDimsAliasSeq] to get the array (which won't allocate if it's bound to a static array symbol i.e. in the call to slice). I'm a bit worried about people creating a lot of garbage accidentally when manipulating dimensions though.

I think the 1D case should perhaps stay working without being an array, would seem like unnecessary noise to add [] there.

9il commented 8 years ago

But all other functions ? Like transposed and iotaSlice

John-Colvin commented 8 years ago

i think yes. Consistency is good here. Also, being able to add extra arguments in the future will help maintain flexibility in the future (very important, particularly once ndslice is out of std.experimental)

9il commented 8 years ago

Ping DCV contributors @ljubobratovicrelja @henrygouk @DmitryOlshansky. What is your opinion about this breaking change?

ljubobratovicrelja commented 8 years ago

I personally like the variadic approach, and would not forcibly replace it with static array. Say for the slice!T, as tensor allocation - most of the time I'd just allocate a tensor, without explicit value initialization - so [] would bother me in syntax mostly. Also, for e.g. I feel having [] in transposed would be a bit odd, in contrast to the present API.

I think I'm already used to the present API, which mostly works with variadics, and I haven't had any trouble with it - I really cannot tell how I would feel with newly proposed syntax. That's just my subjective view on it.

But, obviously since python (numpy) users feel the inconsistency, I suppose there's a real trouble here. Might be good idea to ping some more people, or even have the forum thread about it? (or maybe not the forum thread since we've seen how the simplest debate can burst into flames there...)

John-Colvin commented 8 years ago

Hmm I think actually there's a distinction between arguments that are shapes (e.g. argument to sliced) and arguments that are dimension indices (e.g. arguments to transposed). I think static arrays are better for shapes. I'm undecided on dimension indices but would err on the side of using veridical, if only to make the distinction clear.

9il commented 8 years ago

Forum voting/discussion http://forum.dlang.org/thread/lxsuvoxqjiecjenrfvtj@forum.dlang.org

9il commented 8 years ago

According to the forum discussion (appr. equal Yes/No and small activity) I close this issue with wontfix.