adcomp icon indicating copy to clipboard operation
adcomp copied to clipboard

Add row method to array

Open 2039 opened this issue 5 years ago • 3 comments

I found it easier to do array.row(n) than array.transpose().col(n).

Note: I'm inexperienced in C/C++. I've only tested this on my own program, and it appears to be working, but it should preferably be reviewed first. Please let me know of any mistakes so I can correct them.

2039 avatar May 24 '20 12:05 2039

@2039 Thanks for the suggestion, but unfortunately it's not that simple to add a row method - at least not with the current array design.

To be consistent with other classes (vectors and matrices) the array extractors must provide write access. See e.g. here: http://kaskr.github.io/adcomp/matrix_arrays_8cpp-example.html

a1.col(1) = v1;  // assign v1 to 2nd col of a1
REPORT(a1);

This only works for array columns because columns are contiguous segments of the underlying data.

kaskr avatar May 25 '20 09:05 kaskr

Unless I'm missing something, arrays are already inconsistent with matrices

array<int> A(2,2);
A << 1, 2, 3, 4;
A.transpose() << 5, 6, 7, 8;
std::cout << "A:\n" << A << std::endl;
/*
A:
1
2
3
4
*/

matrix<int> M(2,2);
M << 1, 2, 3, 4;
M.transpose() << 5, 6, 7, 8;
std::cout << "M:\n" << M << std::endl;
/*
M:
5 6
7 8
*/

introducing more inconsistencies should probably be avoided, but I'd argue that the lack of a row accessor is an inconsistency by itself.

2039 avatar May 25 '20 11:05 2039

It's true there are some inconsistencies in the array class that would be nice to fix. However, what I'm saying is that it's not so easy with the current design. I'd be happy to consider a row extractor if you make it in a way that triggers a compile time error if a user tries to assign directly to the array row. Likewise, it would be nice to fix the array transpose() caveat you pointed out, either by implementing the assignment method, or by generating a compile time error if used.

kaskr avatar May 25 '20 14:05 kaskr