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

use different package for ndarray #32

Open
1 of 4 tasks
xnought opened this issue Nov 15, 2022 · 5 comments
Open
1 of 4 tasks

use different package for ndarray #32

xnought opened this issue Nov 15, 2022 · 5 comments
Assignees
Labels
enhancement ✨ New feature or request on hold

Comments

@xnought
Copy link
Member

xnought commented Nov 15, 2022

Move to stdlib.js array

  • abstract this away in other class
  • reimplement prefix sum
  • reimplement slicing
  • reimplement subtraction across slices
@xnought xnought added the enhancement ✨ New feature or request label Nov 15, 2022
@xnought xnought self-assigned this Nov 15, 2022
@xnought
Copy link
Member Author

xnought commented Jan 20, 2023

This can now be translated into whatever format by just reimplementing the falconArray.ts file. Right now its still in NdArray js format

@kgryte
Copy link

kgryte commented Feb 10, 2023

Hello! Stopping by from stdlib-land. One small comment regarding the falconArray abstraction layer. I'd recommend migrating to stdlib property naming conventions (see ndarray/ctor). In particular, instead of the scijs convention of stride, use strides, which is what is used by TF.js and stdlib (and NumPy for that matter). And instead of size, I'd recommend length, which is the stdlib convention.

Thinking ahead to the future, these suggestions are intended to ensure that future contributors see similar idioms in the Falcon ndarray abstraction layer provided they are already familiar with stdlib (and other) ndarrays.

@xnought
Copy link
Member Author

xnought commented Feb 12, 2023

Great points altogether. I'll make those changes. Thanks for the response and links!

I do have a weird attachment to size. I have to reflect why I have that proclivity before I change that to length

@kgryte
Copy link

kgryte commented Feb 12, 2023

Understandable. For context, the PyData convention largely stems from len(x) returning x.shape[0] (similar to nested lists), which is something which is considered a mistake for ndarrays. Thus, there was a need for a different attribute (e.g., size) to indicate the total number of elements.

For stdlib, we wanted to maintain consistency with existing JS convention for array-like objects; namely, using length to indicate how many elements an object contains. If you wanted to, you could keep both and just reflect size to length.

Regardless, while not strictly assumed by stdlib, we cannot guarantee that we won't check for a .length property as a shorthand for numel(shape) in various ndarray functions, so your mileage may vary.

@xnought
Copy link
Member Author

xnought commented Feb 12, 2023

Aha that makes sense!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement ✨ New feature or request on hold
Projects
None yet
Development

No branches or pull requests

3 participants