The Wayback Machine - https://web.archive.org/web/20200701191254/https://github.com/stdlib-js/stdlib/issues/268
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ndarray with column-major ordering cuts across array-of-arrays input #268

Open
rreusser opened this issue Sep 21, 2019 · 3 comments
Open

Ndarray with column-major ordering cuts across array-of-arrays input #268

rreusser opened this issue Sep 21, 2019 · 3 comments

Comments

@rreusser
Copy link
Member

@rreusser rreusser commented Sep 21, 2019

Checklist

Please ensure the following tasks are completed before filing a bug report.

  • Read and understood the Code of Conduct.
  • Searched for existing issues and pull requests.

Description

Description of the issue.

The following code produces an array whose shape does not match what I'd expect:

x = stdlib.array([[1, 2], [3, 4], [5, 6]], { order: 'column-major' })
x.shape
// => [3, 2]

As a matrix, it looks like:

[ 1 4 ]
[ 2 5 ]
[ 3 6 ]

The resulting matrix seems to cut across the array-of-arrays input. If you simply interpret this as a 2 x 3 array instead of 3 x 2, then it matches what I'd expect. I suspect but haven't confirmed that reversing the order of the inferred shape may solve the issue (for 2D and higher dimensionalities). See example and demonstration that reversing may fix here

Related Issues

Does this issue have any related issues?

No.

Questions

Any questions for reviewers?

No.

Other

Any other information relevant to this issue? This may include screenshots, references, stack traces, sample output, and/or implementation notes.

Demo

If relevant, provide a link to a live demo.

For a live demo of the issue, see

Reproduction

What steps are required to reproduce the unexpected output?

See above for code.

Expected Results

What are the expected results?

I expect the inner arrays of array-of-arrays input to show up in the constructed array as either a row or a column, not to be split and spread across multiple columns.

Actual Results

What are the actual results?

shape seems reversed in a manner that spreads inner arrays across multiple columns.

Environments

What environments are affected (e.g., Node v0.4.x, Chrome, IE 11)? If Node.js, include the npm version, operating system, and any other potentially relevant platform information.

The following environments are affected:

  • Usage in Chrome with stdlib-tree.
@kgryte
Copy link
Member

@kgryte kgryte commented Sep 21, 2019

@rreusser Thanks for filing this issue. Will look into producing a fix shortly.

@kgryte kgryte added bug math labels Sep 21, 2019
@kgryte
Copy link
Member

@kgryte kgryte commented Sep 23, 2019

@rreusser Okay, so the current behavior is intentional. As documented in the package documentation, when you specify an order, you are saying what format the data is already in, not what order you want the data to be.

It is not clear, at least in my mind, what a user means when s/he provides a nested array and then declares that the data "order" is column-major, at least in the "descriptive" sense, as the inferred shape based on the provided argument is at odds with, as you show in your examples, the shape as might be expected for data provided as column-major.

Maybe it makes more sense to throw an error here, instead of trying to guess what a user intends?

@kgryte
Copy link
Member

@kgryte kgryte commented Sep 23, 2019

Based on Gitter discussion, consensus seems to be that the README documentation should include an example explicitly showing how order specification is descriptive, not prescriptive, and highlight potential "gotchas", such as specifying "column-major" does not mean that a provided nested array should be flattened to column-major order, but rather that the data is provided in column-major order already.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
You can’t perform that action at this time.