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

[RFC]: achieve ndarray API parity with built-in JavaScript arrays #2656

Open
14 of 47 tasks
kgryte opened this issue Jul 24, 2024 · 15 comments
Open
14 of 47 tasks

[RFC]: achieve ndarray API parity with built-in JavaScript arrays #2656

kgryte opened this issue Jul 24, 2024 · 15 comments
Labels
difficulty: 4 Likely to be moderately difficult. Feature Issue or pull request for adding a new feature. JavaScript Issue involves or relates to JavaScript. Needs Discussion Needs further discussion. priority: High High priority concern or feature request. RFC Request for comments. Feature requests and proposed changes.

Comments

@kgryte
Copy link
Member

kgryte commented Jul 24, 2024

Description

This RFC proposes achieving ndarray API parity with built-in JavaScript arrays. Built-in JavaScript Array and TypedArray objects have a number of methods for searching, manipulating, sorting, and transforming array data.

The goal of this RFC is to add functional APIs providing equivalent functionality for ndarrays. By providing these APIs, stdlib can offer a powerful toolset using a similar vocabulary and interface design as existing art for working with ndarrays. This should help reduce the barrier to ndarray adoption and encourage their more widespread use.

Note, however, that ndarrays have considerable additional complexity due to their multi-dimensional nature, and, in particular, element-wise iteration requires specialized kernels for handling non-contiguous underlying data.

There does exist precedent in stdlib for such kernels (e.g., ndarray/base/assign, ndarray/base/nullary, and ndarray/base/unary). Those packages also provide C APIs which may or may not be relevant to the functional APIs proposed in this RFC.

What follows is an initial list of Array.prototype.* methods and notes regarding whether an equivalent already exists or what constraints we need to consider when designing ndarray equivalent packages.

Top-level: ndarray/*

  • Array.prototype.at

    • ndarray/at
  • Array.prototype.concat

  • Array.prototype.copyWithin

    • specify axis
    • how would this work for strided arrays?
    • for ideal case, would prefer delegating to array prototype method, as likely faster; for non-ideal case, would need to take special care to avoid overwriting accessed values.
  • Array.prototype.entries

    • ndarray/iter/entries
  • Array.prototype.every

    • reduction; specify one or more axes
  • Array.prototype.fill

    • can leverage ndarray/base/assign
    • would be good to have a native add-on to allow for acceleration in Node.js.
  • Array.prototype.filter

    • output one-dimensional array
    • this is a tricky method, as basically need to do a first pass to see which elements pass a predicate function and storing the indices in a temporary buffer, then allocate an output array, and finally do a second pass where we only "take" the unmasked elements and store in the output array
  • Array.prototype.find

    • reduction; specify one or more axes
  • Array.prototype.findIndex

    • reduction; specify one or more axes
  • Array.prototype.findLast

    • reduction; specify one or more axes
  • Array.prototype.findLastIndex

    • reduction; specify one or more axes
  • Array.prototype.flat

    • reshape
    • likely name flatten
    • would we support a depth argument?
    • could potentially flatten subarrays (i.e., flatten "stacks")
  • Array.prototype.flatMap

    • likely name flatten-by
    • would we support a depth argument?
  • Array.prototype.forEach (in-progress)

    • wrap "base" implementation
  • Array.prototype.includes

    • reduction; specify one or more axes
  • Array.prototype.indexOf

    • reduction; specify one or more axes
  • Array.prototype.join

    • needs R&D
    • how would dimension separators work? a different separator for each dimension? configurable?
  • Array.prototype.keys

    • ndarray/iter/indices
  • Array.prototype.lastIndexOf

    • reduction; specify one or more axes
  • Array.prototype.map (in progress)

    • wrap "base" implementation
    • return ndarray having same dtype or 'generic'
  • Array.prototype.pop

    • specify the axis along which to remove?
    • this would essentially be slice(0,axis.length-1)
    • would we return the removed slice?
      • not clear. In theory, you'd want to return the new view, but could also return a two-element array (tuple) with the first element being the view and the second element the "popped" view.
  • Array.prototype.push (???)

    • this would essentially be concat along an axis
    • support for broadcasting?
    • would require a data copy, no mutation
  • Array.prototype.reduce

    • reduction; specify one or more axes
  • Array.prototype.reduceRight

    • reduction; specify one or more axes
    • iterate in reverse direction
  • Array.prototype.reverse

    • create view
    • can potentially wrap "base"
  • Array.prototype.shift

    • specify the axis to remove?
    • this would essentially be slice(1)
    • would we return the removed slice?
      • not clear. In theory, you'd want to return the new view, but could also return a two-element array (tuple) with the first element being the view and the second element the "popped" view.
  • Array.prototype.slice

    • ndarray/slice
  • Array.prototype.some

    • reduction; specify one or more axes
  • Array.prototype.sort

    • specify one or more axes
    • mutate?
      • yes, IMO, as can have to-sorted by the copy version.
  • Array.prototype.splice

    • needs R&D
    • would involving slicing, concatenation, and data copy
  • Array.prototype.toLocaleString

    • needs R&D
  • Array.prototype.toReversed

    • data copy
    • can potentially wrap "base"
  • Array.prototype.toSorted

    • specify one or more axes
    • data copy
  • Array.prototype.toSpliced

    • needs R&D
  • Array.prototype.toString

    • just call ndarray.toString()?
  • Array.prototype.unshift

    • this would essentially be concat along an axis
    • support for broadcasting?
    • would require a data copy, no mutation
  • Array.prototype.values

    • ndarray/iter/values
  • Array.prototype.with

    • requires data copy
    • should be a straightforward application of allocation, call assign in order to copy, and then set the individual element
    • ideally, would also include a native add-on which calls to ndarray/base/assign for acceleration in Node.js

Base: ndarray/base/*

  • Array.prototype.fill

    • ndarray/base/fill
  • Array.prototype.forEach

    • ndarray/base/for-each
  • Array.prototype.map

    • ndarray/base/map
  • Array.prototype.reverse

    • ndarray/base/reverse
  • Array.prototype.slice

    • ndarray/base/slice
  • Array.prototype.toReversed

    • ndarray/base/to-reversed

Related Issues

None.

Questions

No.

Other

No.

Checklist

  • I have read and understood the Code of Conduct.
  • Searched for existing issues and pull requests.
  • The issue name begins with RFC:.
@kgryte kgryte added RFC Request for comments. Feature requests and proposed changes. Feature Issue or pull request for adding a new feature. Needs Discussion Needs further discussion. priority: High High priority concern or feature request. JavaScript Issue involves or relates to JavaScript. difficulty: 4 Likely to be moderately difficult. labels Jul 24, 2024
@kgryte
Copy link
Member Author

kgryte commented Jul 24, 2024

cc @headlessNode

@SarthakPaandey
Copy link
Contributor

I want to work on this issue @kgryte and could you elaborate on the specific goals of aligning ndarray APIs with JavaScript arrays? How will this benefit users and developers?

@kgryte
Copy link
Member Author

kgryte commented Jul 26, 2024

How will this benefit users and developers?

This is already addressed in the OP. Second paragraph.

@SarthakPaandey
Copy link
Contributor

Yup! Starting work on this!

@kgryte
Copy link
Member Author

kgryte commented Jul 26, 2024

@SarthakPaandey Before you begin, it would be best to communicate what you're planning to work on. Some of the above routines are easier than others and should be addressed first. Reductions require R&D and should only be worked on once we've determined the best approach.

@SarthakPaandey
Copy link
Contributor

"Understood, @kgryte. I'll prioritize the routines that are comparatively easier and communicate my plan before starting. I'll defer work on reductions until we've established the best approach. Thanks for the guidance!

@headlessNode
Copy link
Contributor

@kgryte I have started to work on the map method. I plan to submit PR for it when I've implemented the kernels for it up to 3d. This way, it would be possible to get feedback from you earlier and it would be easier for me to work on the feedback. Does that seem reasonable?

@kgryte
Copy link
Member Author

kgryte commented Jul 26, 2024

@headlessNode That makes sense. I am currently working on for-each, which is related.

@kgryte
Copy link
Member Author

kgryte commented Jul 26, 2024

@headlessNode I suggest working a "base" implementation of map first. For @stdlib/ndarray/base/map, we can assume that an output array is provided, similar to ndarray/base/unary. In fact, unary is a good reference for map. The main difference being the support of a thisArg and no C implementation.

@headlessNode
Copy link
Contributor

headlessNode commented Jul 26, 2024

@kgryte The thisArg in our case would be similar to how it is handled in the Array.prototype.map right? e.g.

//Array.prototype.map
let numbers = [1, 2, 3];
let obj = { multiplier: 2 };

numbers.map(function(num) {
  return num * this.multiplier;
}, obj);```

@kgryte
Copy link
Member Author

kgryte commented Jul 26, 2024

@headlessNode Yes. As an example, see https://github.com/stdlib-js/stdlib/tree/develop/lib/node_modules/%40stdlib/array/base/any-by.

@kgryte
Copy link
Member Author

kgryte commented Jul 27, 2024

@headlessNode Pushed up a POC implementation of for-each: https://github.com/stdlib-js/stdlib/tree/develop/lib/node_modules/%40stdlib/ndarray/base/for-each

@headlessNode
Copy link
Contributor

Thanks!

kgryte added a commit that referenced this issue Aug 16, 2024
PR-URL: #2715
Ref: #2656
Co-authored-by: Athan Reines <[email protected]>
Reviewed-by: Athan Reines <[email protected]>
@headlessNode
Copy link
Contributor

Heads up. I have started to work on ndarray/base/fill

@kgryte
Copy link
Member Author

kgryte commented Aug 20, 2024

Sounds good, @headlessNode! Thanks for the heads up!

gunjjoshi pushed a commit to gunjjoshi/stdlib that referenced this issue Aug 21, 2024
PR-URL: stdlib-js#2715
Ref: stdlib-js#2656
Co-authored-by: Athan Reines <[email protected]>
Reviewed-by: Athan Reines <[email protected]>
kgryte added a commit that referenced this issue Sep 7, 2024
PR-URL: #2861
Ref: #2656
Co-authored-by: Athan Reines <[email protected]>
Reviewed-by: Athan Reines <[email protected]>
kgryte added a commit that referenced this issue Sep 7, 2024
PR-URL: #2817
Ref: #2656
Co-authored-by: Athan Reines <[email protected]>
Reviewed-by: Athan Reines <[email protected]> 
Signed-off-by: Muhammad Haris <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: 4 Likely to be moderately difficult. Feature Issue or pull request for adding a new feature. JavaScript Issue involves or relates to JavaScript. Needs Discussion Needs further discussion. priority: High High priority concern or feature request. RFC Request for comments. Feature requests and proposed changes.
Projects
None yet
Development

No branches or pull requests

3 participants