Skip to content

test: add tests to ndarray/base/nullary #2609

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

Merged
merged 2 commits into from
Jul 19, 2024

Conversation

headlessNode
Copy link
Member

Description

What is the purpose of this pull request?

This pull request:

  • add tests to @stdlib/ndarray/base/nullary/test/test.4d.js for 100% test coverage

Related Issues

Does this pull request have any related issues?

This pull request:

Questions

Any questions for reviewers of this pull request?

No.

Other

Any other information relevant to this pull request? This may include screenshots, references, and/or implementation notes.

No.

Checklist

Please ensure the following tasks are completed before submitting this pull request.


@stdlib-js/reviewers

@headlessNode
Copy link
Member Author

@kgryte In 3d, the incorrect strides were causing the offset and minmax indices to go out of bounds. I made some adjustments to the strides to ensure that the offset and minmax indices are within the buffer bounds. Hopefully, everything is now correct. Please review, thanks.

@kgryte kgryte added the Tests Pull requests specifically adding tests. label Jul 18, 2024
@kgryte
Copy link
Member

kgryte commented Jul 18, 2024

In 3d, the incorrect strides were causing the offset and minmax indices to go out of bounds.

@headlessNode To clarify, your comment concerns 3d or 4d?

@headlessNode
Copy link
Member Author

@kgryte both. I tried different combinations of strides with 4d (for my understanding) and came to the above conclusion.

@kgryte
Copy link
Member

kgryte commented Jul 19, 2024

both. I tried different combinations of strides with 4d (for my understanding) and came to the above conclusion.

Still not sure I follow. Are you saying that there are bugs in the 3d tests that need to be fixed? Or that you needed to make modifications to the 3d tests to make them work for 4d?

In the current PR, you only have 4d test changes, so I am assuming the latter, but your comments suggest otherwise.

@kgryte
Copy link
Member

kgryte commented Jul 19, 2024

@headlessNode Did a review. Some of the strides were off, but I've now fixed these.

It may be worthwhile if you use the stdlib REPL to help inform the strides.

$ make repl

Once in the REPL,

In [1]: var shape2strides = require( '@stdlib/ndarray/base/shape2strides' );

In [2]: var blockSize = require( '@stdlib/ndarray/base/nullary-tiling-block-size' );

In [3]: var dt = 'float64';

In [4]: var bsize = blockSize( dt )
Out[4]: <number>

In [5]: shape2strides( [ bsize*2, 2, 1, 2 ], 'row-major' )
Out[5]: [ 4, 2, 2, 1 ]

In [6]: shape2strides( [ bsize*2, 2, 1, 2 ], 'column-major' )

From there, you can scale the strides as necessary to get non-contiguity, as desired.

Copy link
Member

@kgryte kgryte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks, @headlessNode!

@kgryte kgryte merged commit e4cd553 into stdlib-js:develop Jul 19, 2024
6 checks passed
@headlessNode
Copy link
Member Author

@kgryte Thanks for the review and suggestion. Also sorry for the confusion, I should've phrased it differently. There's no bug, I was sharing my "findings" about why some strides work and some don't.

@headlessNode headlessNode deleted the nullary-tests-4d branch July 19, 2024 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Tests Pull requests specifically adding tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants