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

aggregation operations on 2d arrays using axis 1 don't return an array type and return only MathScalarType #3160

Open
jeffzyliu opened this issue Feb 19, 2024 · 3 comments

Comments

@jeffzyliu
Copy link

jeffzyliu commented Feb 19, 2024

Describe the bug
On functions like mean, min, max, std, if you take some 2d number array and want to aggregate it along the 1 axis to retrieve a 1d array of output, you want to get some kind of array/matrix/vector type back. However, it only returns MathScalarType which causes a typescript error. I believe that this code in the types/index.d.ts file at line 2668 causes this:

  /**
   * @param A A single matrix
   * @param dimension The mean over the selected dimension
   * @returns The mean value
   */
  mean<T extends MathScalarType>(
    A: T[] | T[][],
    dimension?: number | BigNumber
  ): T

This has a return value of T, but it actually returns T[] when you have dimension=1 and we want the annotated type to match.

To Reproduce
something like

const dataArray: number[][] = [[1, 2], [3, 4]];
const resultArray: number[] = math.mean(dataArray, 1);

should create an error saying "Type 'MathScalarType' is not assignable to type 'number[]'". But this works perfectly as intended and does actually return the answer correctly.

This happens on mean, min, max, std, and maybe more things that involve axis but I haven't tested them.

@josdejong
Copy link
Owner

Thanks for reporting. Anyone able to help improve these type definitions?

@rakheesingh
Copy link

I wanted to take this up but @josdejong didn't get exact issue, can you help me here ?

@josdejong
Copy link
Owner

Thanks @rakheesingh .

To solve this issue, we need to go through all statistical functions like mean that have an optional property dimension, and change the type definition such that it matches the actual behavior. When dimension is provided, the returned type will not be a scalar but a matrix/array (just try it out to see what happens :) ).

For example:

const A = [[1, 2], [3, 4]]
math.mean(A)                 // number 2.5
math.mean(A, 1)              // Array  [ 1.5, 3.5 ]
math.mean(math.matrix(A), 1) // Matrix [ 1.5, 3.5 ]

Some pointers:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants