Not possible to map cube root `cbrt( )`

[dvd101x]

On version 11.5.0 while evaluating map([1, 8, 27], cbrt)

yields an error
TypeError: Unexpected type of argument in function cbrt (expected: boolean, actual: Array, index: 1)

On version 10.6.4

cbrt([1, 8, 27]) yields [1, 2, 3] as expected for that version.

but map([1, 8, 27], cbrt) yields the same error TypeError: Unexpected type of argument in function cbrt (expected: boolean, actual: Array, index: 1)

This behavior is unique for cbrt( ) and doesn't happen for other functions that droped element wise matrix support

If we try the square root map([1 ,4, 9], sqrt) it yields [1, 2, 3] as expected

To Reproduce

On version 11.5.0 evaluate map([1, 8, 27], cbrt)

[dvd101x]

This works somehow map([1, 8, 27], _(x) = cbrt(x))

[josdejong]

The error thrown when executing map([1, 8, 27], cbrt) (wheras map([1, 8, 27], sin) works fine) has the following cause: the callback argument of map is executed with three arguments: the value, the index, and the parent matrix. This would invoke for example cbrt(27, [2], [1, 8, 27]) and sin(27, [2], [1, 8, 27]). However, the functions cbrt and sin do not have these second and third arguments The math.map function will try to find a matching signature keeping the second and third argument optional. That succeeds in the case of sin. But in the case of cbrt it tries to use the math.cbrt(x, allRoots) signature, which is wrong. It would be nice to improve on this. The logic can be found here:

mathjs/src/function/matrix/map.js

Lines 77 to 84 in 224ef38

	// invoke the callback function with the right number of arguments
	if (args === 1) {
	return callback(value)
	} else if (args === 2) {
	return callback(value, index)
	} else { // 3 or -1
	return callback(value, index, array)
	}

I think we can improve on this by first checking if the callback is a typed-function. If so, we can inspect whether there is a matching signature for either 1, 2, or 3 arguments via typed.find(callback, ...). Depending on that, we can invoke the found callback with 1, 2, or 3 arguments.

For now, a workaround is indeed to to explicitly only pass one argument to cbrt:

map([1, 8, 27], _(x) = cbrt(x))

[dvd101x]

Recently I found out it's also possible to do it like

map([1, 8, 27], cbrt(_))

Is it ok if I attempt to make a PR about this?

[josdejong]

Ah, that is nice indeed!

Yes a PR improving the behavior of map would be welcome! (or also: documenting this notation map([1, 8, 27], cbrt(_)))

[dvd101x]

Hi Jos, I've been working on it and even though I'm having some progress at the moment I'm stuck.

I tried to replace this:

mathjs/src/function/matrix/map.js

Lines 77 to 84 in 224ef38

	// invoke the callback function with the right number of arguments
	if (args === 1) {
	return callback(value)
	} else if (args === 2) {
	return callback(value, index)
	} else { // 3 or -1
	return callback(value, index, array)
	}

With this:

if (typed.isTypedFunction(callback)) {
    // invoke the callback function with the right number of arguments
    if (args === 1) {
        try {
            return typed.find(callback, [typeOf(value)])(value)
        } catch (err) {
        }
    } else if (args === 2) {
        try {
            return typed.find(callback, [typeOf(value), typeOf(index)])(value, index)
        } catch (err) {
        }
    } else { // 3 or -1
        try {
            return typed.find(callback, [typeOf(value), typeOf(index), typeOf(array)])(value, index, array)
        } catch (err) {
        }
    }
} else {
    // invoke the callback function with the right number of arguments
    if (args === 1) {
        return callback(value)
    } else if (args === 2) {
        return callback(value, index)
    } else { // 3 or -1
        return callback(value, index, array)
    }
}

So now math.map([1, 8, 27], math.cbrt) yields [undefined, undefined, undefined] instead of TypeError: map attempted to call 'cbrt(1,[0])' but argument 2 of type Array does not match expected type boolean

Sadly map([1, 8, 27], cbrt still yieldsTypeError: Unexpected type of argument in function cbrt (expected: boolean, actual: Array, index: 1)

Any pointers would be greatly appreciated

[josdejong]

Thanks for working on this David!

I think the issue here is trying to match the arguments against args (holding the max argument count). What we should do here I think is:

• See if there is a matching signature for callback(value, index, array), if so, use that
• See if there is a matching signature for callback(value, index), if so, use that
• See if there is a matching signature for callback(value), if so, use that
• Else, throw an error

Maybe typed.resolve is easier to use here then typed.find? Then you don't need all the typeOf calls.

I did a bit of testing, and the following seems to work like we want (not tested thoroughly though, and I didn't check any performance impact):

if (typed.isTypedFunction(callback)) {
  // invoke the callback function with the right number of arguments

  const signature3 = typed.resolve(callback, [value, index, array])
  if (signature3) {
    return signature3.implementation(value, index, array)
  }

  const signature2 = typed.resolve(callback, [value, index])
  if (signature2) {
    return signature2.implementation(value, index)
  }

  const signature1 = typed.resolve(callback, [value])
  if (signature1) {
    return signature1.implementation(value)
  }

  // TODO: more extensive error message
  throw new TypeError('No matching signature found in callback function "' + typed.name + '"')
} else {
  // invoke the callback function with the right number of arguments
  if (args === 1) {
    return callback(value)
  } else if (args === 2) {
    return callback(value, index)
  } else { // 3 or -1
    return callback(value, index, array)
  }
}

[dvd101x]

Hi Jos,

I tried your solution and it works really well, I tried some tweaks but didn't show any improvement.

I thought on an improvement to the function map regarding multiple input arguments and broadcasting, but it might be a separate discussion.

[dvd101x]

I did validate your solution and included a simple test, everything seems to work. I'm not familiar with git, I have your solution ready for a pull request but I don't know if I should. As it's yours.

[josdejong]

👍 thanks for validating David, I'll work out a PR and some tests.

[dvd101x]

Thank you Jos!