rust-ndarray / ndarray

ndarray: an N-dimensional array with array views, multidimensional slicing, and efficient operations
https://docs.rs/ndarray/
Apache License 2.0
3.56k stars 299 forks source link

Add matlab-like tril method for two-dimensional arrays #1331

Open fedemagnani opened 1 year ago

fedemagnani commented 1 year ago

Adding trill method for two-dimensional arrays in order to create a lower triangular matrix from a given matrix. This function reproduces the behavior of the Matlab function with same name: it returns the elements on and below the kth diagonal of A

nilgoyette commented 1 year ago

I'm not against adding this feature to ndarray and I think the other maintainers would agree. This being said

adamreichold commented 1 year ago

I'm not a fan of the name. tril seems to be the standard (Matlab and Numpy), but it means nothing (maybe it's just me?)

I guess we could just spell it out, e.g. triangular_lower?

fedemagnani commented 1 year ago

I wrote this method trying to reproduce the behaviour of a matlab script which was invoking this function.

  • I'm not a fan of the name. tril seems to be the standard (Matlab and Numpy), but it means nothing (maybe it's just me?)

Like you I found the name a bit confusing at first glance, but since it generates a lower triangular from a given matrix I thought that "tri" was standing for "triangular" and "l" as "lower". I decided to keep the same name for consistency with the matlab function thinking that it could have been useful for people with my similiar use case.

  • I didn't take the time to think about it, but the implementation looks like it could be "better". For example, numpy does it in 3 lines (I know we can't and shouldn't do it like Numpy)

Regarding the length of the code, this is the way I wrote it and I think that adding a method which expands the functionalities of the package is better than not doing it :)

I don't think that this method will be used that much but if it will surge the need of optimizing it I bet that it is going to be optimized in the future

nilgoyette commented 1 year ago

I don't think that this method will be used that much but if it will surge the need of optimizing it I bet that it is going to be optimized in the future

There's no need to procrastinate, we can do it properly right now. If you're still interested, of course.