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

Dependencies executing function #197

Open
eramirez11 opened this issue Oct 7, 2019 · 6 comments
Open

Dependencies executing function #197

eramirez11 opened this issue Oct 7, 2019 · 6 comments

Comments

@eramirez11
Copy link

eramirez11 commented Oct 7, 2019

Hello, I have worked with Dentaku before and i am getting this behaviour

dentaku = Dentaku::Calculator.new
dentaku.add_function(:do_raise, :numeric, ->{raise 'Hey'; 1})
dentaku.dependencies('IF(do_raise() , 1, 0)')

'RuntimeError: Hey'

I expected than dependencies just get me an empty array, but it executes the function inside it.
This behaviour only appears when I use function 'IF', so I suspected something maybe change an so it did.

It appears to be this commit fault 'short-circuit IF function evaluation' b79a366

I revert those changes in a clone of the repo and It worked as expected

@rubysolo
Copy link
Owner

rubysolo commented Oct 8, 2019

I think this is to be expected with short-circuiting. For short-circuiting to work when calculating dependencies, we have to evaluate the predicate so that we know which branch to take. The old behavior just returned all dependencies of the predicate and the true and false expressions. The benefit of the current implementation is that we can evaluate something like

IF(true, 5, x)

when we don't have a value for x.

Can you talk a little more about the use case you're trying to solve?

@eramirez11
Copy link
Author

Sorry for taking so long to answer, and thank you for addressing my issue

Well my problem is the following, I have a function, I need to know the dependencies before executing because the function does not have all its values set

This is an example, on what is happening

  dentaku = Dentaku::calculator
  dentaku.add_function(:get_value, :numeric, ->(multiplier){@value * multiplier})
  formula = "IF(get_value(1) == 1, 1, 2) + category"
  dentaku.dependencies(formula)
  # Dentaku::Error: undefined method `*' for nil:NilClass

The problem here is that I am only expecting the dependencies and I call a function witch isn't ready for executing.

@rubysolo
Copy link
Owner

rubysolo commented Nov 19, 2019

I think the some of the problem is that functions in Dentaku are implemented in Ruby, so Dentaku is unable to see what their dependencies are. Is it possible to restructure your function so that all the dependent variable values are passed in as arguments at call time? Something like:

dentaku = Dentaku::calculator
dentaku.add_function(:get_value, :numeric, ->(multiplier, value) { value * multiplier })
formula = "IF(get_value(1, value) == 1, 1, 2) + category"
dentaku.dependencies(formula)
# => ["value", "category"]

Then you could assign value to @value in the calculator memory or the environment hash passed to evaluate.

@value = 10
dentaku.evaluate(formula, category: 7, value: @value)
# => 9

@eramirez11
Copy link
Author

This is unfortunate because I am working with an API, and I need to ask wich values I am going to use before actually using it, so I have no way to know every parameter, if I do it, the request can take a very long time defeating all purpose of using the method dependencies.

@fsateler
Copy link
Contributor

fsateler commented Nov 21, 2019

I think the some of the problem is that functions in Dentaku are implemented in Ruby, so Dentaku is unable to see what their dependencies are.

Indeed, that is our problem (I work with @eramirez11).

For short-circuiting to work when calculating dependencies, we have to evaluate the predicate so that we know which branch to take

This effectively requires evaluating while parsing, which doesn't feel right to me.

Our use case is the following. We allow our users to define their own expressions, which will be used later (this is the "API" we expose). Not only later, but many values are not available at expression definition time (that is, we allow creating "formulas", which we then associate with various domain models from which they obtain their values).

Because the formula evaluation happens at some time in the future, possibly on a background job, we need to validate the formulas before storing them, when the user has a chance to fix them if they have a problem.

As part of validation, we use dependencies to obtain all the used variables, to detect unknown variables. For example, if we provide variable foo but the user typed fooo we can display an error due to the unknown variable fooo.

This short-circuiting logic is preventing us from detecting invalid formulas because we don't have the values at validation time. Moreover, for us any short-circuiting would be wrong, as IF(value, 1, fooo) should be flagged as an invalid formula, even if value happens to be true.

I hope this makes our problem with the current behavior clearer.

@rubysolo
Copy link
Owner

Sorry for the delay on this! I pushed up the branch short_circuit_toggle that would allow you to disable short-circuit evaluation. Would this solve the issue?

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

No branches or pull requests

3 participants