skypy icon indicating copy to clipboard operation
skypy copied to clipboard

Lambda functions can be defined in config

Open JonathanDHarris opened this issue 4 years ago • 6 comments

Description

Allow users to define lambda functions in their configs. Resolve's #318

Checklist

  • [x] Follow the Contributor Guidelines
  • [x] Write unit tests
  • [ ] Write documentation strings
  • [ ] Assign someone from your working team to review this pull request
  • [ ] Assign someone from the infrastructure team to review this pull request

JonathanDHarris avatar Nov 20 '20 12:11 JonathanDHarris

I have a very similar changeset on my machine, which I never propose because of the dangers of eval(). My idea was to only eval the lambda function body:

fdef = 'x**2 + 3'
f = lambda x: eval(fdef, locals())

I think this would severely limit the harm that can be done in the lambda function body. But then I never figured out a good way to specify the function arguments.

ntessore avatar Nov 20 '20 12:11 ntessore

I'm not sure there's anyway to resolve #318 without using eval, so I think a decision needs to be made to use eval or close the issue off as won't fix.

e: just saw your edit, we can go with that if you prefer it. Can you explain how it limits the damage eval can cause? My understanding is the user could still put any arbitrary code in their config and execute it.

JonathanDHarris avatar Nov 20 '20 13:11 JonathanDHarris

It's true (at the moment) that arbitrary functions can be run from config files, and that's another issue. But I wasn't thinking of malicious code rm -rfing the system as much as someone doing something weird that messes up SkyPy internals. Hence why I would prefer to limit the input to be only a single statement.

There is a way to build lambdas programmatically, but it requires work:

>>> lambda_args = ['x', 'y']
>>> lambda_body = 'x**2 + y + 1'
>>>
>>> import ast
>>> args = ast.arguments(posonlyargs=[],
...                      args=[ast.arg(arg) for arg in lambda_args],
...                      kwonlyargs=[],
...                      kw_defaults=[],
...                      defaults=[])
>>> body = ast.parse(lambda_body, mode='eval').body
>>> expr = ast.Expression(ast.Lambda(args, body))
>>> expr.line_no = 1  # from YAML
>>> expr.col_offset = 1  # from YAML
>>> expr = ast.fix_missing_locations(expr)
>>> code = compile(expr, filename='<ast>', mode='eval')
>>> func = eval(code, {})  # lambda works in a vacuum
>>> func(2, 3)
8

And this still leaves the problem of how to obtain the list of arguments and the function body from the YAML.

Edit for your edit about my edit: If the eval() is the body of the lambda, then I believe it can only contain a single expression which cannot be an import statements and such.

ntessore avatar Nov 20 '20 14:11 ntessore

"And this still leaves the problem of how to obtain the list of arguments and the function body from the YAML."

Something like this should work

function_def: !lambda
  x: 1
  b: 2
  function_def:'x**2 + y + 1'

JonathanDHarris avatar Nov 20 '20 16:11 JonathanDHarris

We want the lambdas to produce callables and not values, in order to define parametrisations such as in the examples in a flexible manner.

Since there is no good way to use YAML intrinsics to make this easier, I think the !lambda tag should simply take its value as a scalar node (string) and parse. If we are using Python lambda syntax:

mycallable: !lambda 'x, y: x**2 + y + 1'

Since there's a : in the syntax, we have no way around quotes. However, I have always been fond of Julia's anonymous function syntax:

mycallable: !lambda (x, y) -> x**2 + y + 1

In addition to all of this, we will also need to export symbols such as exp, sin, cos, etc. for the evaluation of the function body. That's quite the big task, so don't feel obligated to tackle this issue.

ntessore avatar Nov 20 '20 17:11 ntessore

On further thought there's even a whole other infrastructure dimension to this, because the lambda's should have access to certain other parts of the pipeline, such as parameters. @JonathanDHarris I will review this now and then we will iterate later on.

ntessore avatar Nov 20 '20 20:11 ntessore