mathjs icon indicating copy to clipboard operation
mathjs copied to clipboard

Import async function

Open moics30 opened this issue 5 years ago • 42 comments

I need to import an async function but have an error because when the function is evaluated return a promise. Is there any way to do that?

Thanks for advance.

moics30 avatar Mar 18 '19 14:03 moics30

Would it be possible to share a small code snippet showing what you are trying to do?

harrysarson avatar Mar 19 '19 18:03 harrysarson

Sure!

const math = require('mathjs');
math.import({
  getExchangeRate: function () {
    return new Promise(resolve => resolve(3.45)); // e.g. Connect to database, or any other async request.
  },
});

const expr = "a = b*getExchangeRate()"; // expression to evaluate

When eval "getExchangeRate", the error occur because the get value is [object Promise]

Thanks for advance.

moics30 avatar Mar 21 '19 15:03 moics30

@moics30 non of the functions of math.js support promises, so if one of the values of multiply is a promise, the function has no clue what to do. You will have to first resolve the promise, store the result in a variable, and use that in your calculations (clearly separate sync and async).

josdejong avatar Mar 21 '19 19:03 josdejong

I'm trying to do something like this:

parser.set('select', async params => {
  const count = await Model.count()

  return count
}

However, when calling the parser const response = parser.evaluate('select()') throws following error:

TypeError: Value has unknown type. Value: [object Promise]

Can we create async functions? Any help on this would be highly appreciated.

CC: @josdejong

atulmy avatar Jan 02 '20 21:01 atulmy

Async functions are not something that mathjs supports. Saying that your example should work - I made a quick example: https://repl.it/repls/GargantuanAcclaimedCoins

const mathjs = require('mathjs');

const parser = new mathjs.parser();

parser.set('select', async params => {
  await Promise.resolve();
  return 17;
})

parser.evaluate('select()')
  .then(console.log);

which does what you would expect it to do :)

harrysarson avatar Jan 02 '20 22:01 harrysarson

That was super quick @harrysarson, thanks!

I tried your example and it does work. However in case of parser.evaluate('select() + 10') it actually fails with the same issue. Any idea how can we achieve that?

Updated example: https://repl.it/repls/UnselfishBrownAlphatest

atulmy avatar Jan 02 '20 22:01 atulmy

The parser has no support for async responses. An async function should be invoke with async (or by resolving the returned promise), and this is not what the parser does.

I think you can simply do something like this:

const mathjs = require('mathjs')

async function run () {
  const parser = new mathjs.parser()

  const count = await Model.count() // first resolve (async)
  parser.set('select', count)       // then set the results (sync)

  const result = parser.evaluate('select()')
  console.log(result)
}

run()

So first you resolve your async value and after that put the results in the parser.

Be careful in your examples not to run any evaluation before your await Model.count() is resolved, until then it will of course be undefined.

josdejong avatar Jan 03 '20 07:01 josdejong

Okay, will use a work around. Thanks for the quick help and building this awesome library @josdejong!

atulmy avatar Jan 03 '20 08:01 atulmy

I do not see this as a workaround but as a proper, valid solution. The Parser simply is not async, and adding support for it would not add anything. I don't think it would make your code simpler or better. It may even make it more tricky and complicated when you mix sync and async on various levels that way: as you can see from my example you have to await for the count to resolve before you can evaluate. It would be very easy to make the mistake to call set without await and then call evaluate as Harry demonstrates with his example :D

josdejong avatar Jan 03 '20 09:01 josdejong

Sorry my previous comment sounds a bit offensive, that was not my intention. Anyway, I think there is no need at this point to add async support and it's easy to write your own wrapper if needed. Will close this question now, feel free to reopen if needed.

josdejong avatar Jan 03 '20 19:01 josdejong

I think async support would really help when we are working with cases where user creates own formula with different params to it and which needs to be resolved from database.

atulmy avatar Jan 03 '20 20:01 atulmy

Atul can you give a few examples of your idea of using async functions inside the parser? Some use-cases that would be very hard, verbose, cumbersome, or simply impossible to achieve without having async support in mathjs itself?

josdejong avatar Jan 04 '20 13:01 josdejong

Hey @josdejong , thank you for considering the request.

I'm building a web app where users can create lists (eg: Cars or Bikes) enter key and values for each item in list (c1 = 10, b1 = 20). Users can later write formula along with passing parameters (keys) for computation:

const formula = `select(cars("c1", "c2"), bikes("b1", "b2")) + 5`

In above formula, c1, c2, b1, b2 are typed by user inside the formula itself which points to some values in database for example (c1 = 10, c2 = 15, b1 = 20, b2 = 25)

Custom function cars:

parser.set('cars', async function () {
  const cars = arguments // holds the parameters passed by user "c1", "c2"
  
  let sum = 0

  for(let c of cars) {
    sum += await Cars.findOne({ name: c }).value
  }

  return sum // will be 10 + 15 = 25
})

Custom function bikes:

parser.set('bikes', function () {
  const bikes = arguments // holds the parameters passed by user "b1", "b2"
  
  let sum = 0

  for(let b of bikes) {
    sum += await Bikes.findOne({ name: b }).value
  }

  return sum // will be 20 + 25 = 45
})

Custom function select:

parser.set('select', function () {
  const sum = 0

  for(let a of arguments) {
    sum += a
  }

  return sum // will be 25 + 45 = 70
})

Evaluating the result:

const parser = mathjs.parser()

const result = parser.evaluate(formula)

console.log(result) // will be 70 + 5 = 75

Looking forward to hearing your thoughts on this.

atulmy avatar Jan 04 '20 15:01 atulmy

Ah, now I understand you Atul. I thought you wanted to resolve the result of an async function and load it in the parser via parser.set, but you have async functions inside the expressions you want to use. That is a valid use case.

I think there is a straightforward way to add async support to mathjs functions. It will be a lot of work though. We can add a new data type Promise to mathjs, and extend every function of mathjs to support Promise input. It's only a few lines, but we need to implement it for every function, and need to add tests and extend the docs.

Here is a working proof of concept (you can achieve this by extending mathjs):

https://runkit.com/josdejong/5e1219882bec72001a472a21

const { create, all, factory } = require('mathjs')

const mathjs = create(all)

const createPromise = factory('Promise', ['typed'], ({ typed }) => {
  // add a new type Promise in mathjs
  typed.addType({
    name: 'Promise',
    test: function (x) {
      return x
        ? typeof x.then === 'function' && typeof x.catch === 'function'
        : false
    }
  })

  // create a conversion to convert from any value to a Promise,
  // so we can mix promise and non-promise inputs to our functions
  typed.addConversion({
    from: 'any',
    to: 'Promise',
    convert: x => Promise.resolve(x)
  })

  return Promise
})

// Create a factory which will extend the function addScalar with support for Promise
// To make this work for real, we should create a Promise implementation for ALL functions
// of mathjs
const createAddPromise = factory('addScalar', ['typed', 'Promise'], ({ typed, Promise }) => {
  return typed('addScalar', {
    'Promise, Promise': function (xPromise, yPromise) {
      return Promise.all([xPromise, yPromise]).then(([x, y]) =>
        mathjs.addScalar(x, y)
      )
    }
  })
})

mathjs.import([createPromise, createAddPromise])

async function getDelayedResult (result, delay = 2000) {
  const resolvedResult = await result // in case result is a Promise

  return new Promise(resolve => {
    setTimeout(() => {
      resolve(resolvedResult)
    }, delay)
  })
}

mathjs.import({
  getDelayedResult
})

function log (...args) {
  console.log([new Date().toISOString(), ...args].join(' '))
}

async function run () {
  log('Starting async calculations...')

  const result1 = await mathjs.add(getDelayedResult(2), 3)
  log('async result1:', result1)

  const result2 = await mathjs.evaluate('20 + getDelayedResult(30)')
  log('async result2:', result2)
  
  const result3 = mathjs.evaluate('5 + 7')
  log('sync result3:', result3)  
  
  const result4 = await mathjs.evaluate('1 + getDelayedResult(getDelayedResult(1))')
  log('async result4:', result4)
}

run()

Let's give it some more thought if this is something we want built-in in mathjs, and investigate how much work it would require.

josdejong avatar Jan 05 '20 17:01 josdejong

You added a conversion from 'any' to 'Promise'. Is there any way to make that conversion work the other direction? For instance, by changing typed-function so that when a promise is encountered in an argument it will wait until it resolves before evaluating the function? I can imagine that that would cause every other function that uses typed-function to become async, such as math.evaluate, so maybe it wouldn't work. But if it did, you could avoid changing every function. Or perhaps you could release a typed-function-async and allow custom builds of math.js to choose whether to include async support, so that it is not a breaking change.

ericman314 avatar Jan 05 '20 17:01 ericman314

You added a conversion from 'any' to 'Promise'. Is there any way to make that conversion work the other direction?

That would be very handy but I think that's not possible, that would require to write a synchronous function which turns async input into sync output.

For instance, by changing typed-function so that when a promise is encountered in an argument it will wait until it resolves before evaluating the function?

That is actually a very good idea! It may be possible to extend typed-function such that it can automatically add signatures to a function to resolve promises. That would save a lot of effort! I can do an experiment with that and see how it works out (see how edge cases work out and see if this has impact on the performance etc).

josdejong avatar Jan 05 '20 19:01 josdejong

That would make functions like math.evaluate have to return a promise, wouldn't it, like in your example? It would be awesome if it could "just work", where if any of the functions is async, evaluate will return a promise, but if not, it just returns the normal value (not a resolved promise) so that it is not a breaking change.

ericman314 avatar Jan 05 '20 23:01 ericman314

@ericman314 no, I was afraid of that myself too (full API having to become async), but that's not the case: sync input -> sync output, async input -> async output. I've added a regular sync example in the code example https://github.com/josdejong/mathjs/issues/1445#issuecomment-570931476 to show that. Try it out for yourself :)

josdejong avatar Jan 06 '20 08:01 josdejong

Okay, that is really cool.

One weird thing, I tried modifying one of the expressions:

const result2 = await mathjs.evaluate('getDelayedResult(20) + getDelayedResult(30)')

And that expression took 2000 ms to return, as expected. But then I tried:

const result2 = await mathjs.evaluate('20 + getDelayedResult(getDelayedResult(30))')

And this expression still only took 2000 ms to evaluate, when I expected it to take 4000 ms.

ericman314 avatar Jan 06 '20 15:01 ericman314

That is an interesting test :smile: . The reason that it still takes 2000 ms instead of 4000 ms is that getDelayedResult is not expecting async input for result. If you first await the result to resolve, it will take 4000 ms. I've updated the example to also handle that case.

josdejong avatar Jan 06 '20 18:01 josdejong

Ah, that makes sense. And most impressive is that the two promises are resolved in parallel even when they are not adjacent in the evaluation tree. This expression still takes 2000 ms:

const result4 = await mathjs.evaluate('(1 + getDelayedResult(2)) + (3 + getDelayedResult(4))') // 2000 ms

So the last remaining question is whether we can automatically add a new signature for each function. In your example, you created a custom Promise type in math.js, including a conversion from any to Promise so you can mix promise and non-promise types. Then if you could have math.js automatically create async versions of each function, that would be a really elegant solution. And as you showed in your example, you wouldn't even have to modify typed-function to do it.

The alternative would be to modify typed-function so that it would resolve promise arguments using await, and then match the resolved types to the correct function signature. Then you wouldn't need to add Promise types to math.js at all, I think. It could be a really useful feature by itself for typed-function, but it removes some generality by assuming the user of typed-function would always want a promise resolved immediately.

ericman314 avatar Jan 06 '20 20:01 ericman314

the two promises are resolved in parallel

Yes :smile: that's because of the Promise.all([...]).

I'll do some experimenting with typed-function, that would be really elegant if it works out. Interesting idea to see if it's possible to call await before every argument in typed-function! Not sure either if that can work out, it's worth a try.

josdejong avatar Jan 06 '20 20:01 josdejong

Hey @josdejong, are you planning to publish support for async functions soon?

atulmy avatar Jan 10 '20 17:01 atulmy

@atulmy I want to do a little experiment with typed-function first to investigate whether there are options there. Depending on that we can estimate how much work it will be and decide whether we want to go for it.

You can make yourself a working solution already though by working out the proof of concept I gave in https://github.com/josdejong/mathjs/issues/1445#issuecomment-570931476 for the functions and operators that are relevant for you.

josdejong avatar Jan 11 '20 18:01 josdejong

@josdejong yes, I'm currently using the solution you have provided in the comment, thank you, it is very helpful. However, the native support one would be cleaner. I'm using following code at the moment:

const mathjs = create(all)

const createPromise = factory('Promise', ['typed'], ({ typed }) => {
  typed.addType({ name: 'Promise', test: function (x) { return x ? typeof x.then === 'function' && typeof x.catch === 'function' : false } })
  typed.addConversion({ from: 'any', to: 'Promise', convert: x => Promise.resolve(x) })
  return Promise
})

const mathFunctionsKeys = ['addScalar', 'multiplyScalar', 'divideScalar', 'subtract']

let asyncFunctions = [createPromise]

for(let k of mathFunctionsKeys) {
  asyncFunctions.push(
    factory(k, ['typed', 'Promise'], ({ typed, Promise }) => {
      return typed(k, { 'Promise, Promise': function (xPromise, yPromise) { return Promise.all([xPromise, yPromise]).then(([x, y]) => mathjs[k](x, y)) }})
    })
  )
}

mathjs.import(asyncFunctions)

atulmy avatar Jan 11 '20 18:01 atulmy

:+1:

josdejong avatar Jan 12 '20 20:01 josdejong

I've worked out a proof of concept for this in typed-function, see: https://github.com/josdejong/typed-function/pull/34. Documentation and examples is still lacking but you can look at the unit tests I created.

Functionally it all works like a charm. I still have to see if this has an impact on performance (both of creating typed-functions as well as using them).

josdejong avatar Jan 18 '20 19:01 josdejong

May I ask what the status on this is? I am currently assessing different libraries and mathjs would have been my choice but I do need async support. Any ETA?

nbst84 avatar May 07 '20 16:05 nbst84

Ah, good point. There is a proof of concept that works like a charm. I still have to see whether there is any impact on the performance. If all is good, what is left over is writing out docs and integrating this in mathjs with a new configuration option (except when there is no performance impact).

I can try pick it up in the coming weeks, it's a very nice feature to have.

josdejong avatar May 07 '20 16:05 josdejong

First benchmarks show there is no performance penalty 🎉 . So that means we will not need to create an option for this in mathjs, and simply can provide async support on all functions for free. Still a lot of things to work out, I took some shortcuts to be able to do these benchmarks and test integration in mathjs.

josdejong avatar May 27 '20 14:05 josdejong