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

Bug: safemath functions not working [with fix] #28

Closed
lchmtw opened this issue Oct 4, 2020 · 2 comments
Closed

Bug: safemath functions not working [with fix] #28

lchmtw opened this issue Oct 4, 2020 · 2 comments

Comments

@lchmtw
Copy link

lchmtw commented Oct 4, 2020

While testing with math functions like 'log' and 'ceil', I have found that the workflow does not work as expected (gives 0 for everything).

Not sure if I am the only one experiencing this, but I was able to fix it by replacing this line (line 12 in converter/safe_math.py):
safe_dict[k] = lambda *a: decimal.Decimal(getattr(math, k)(*a))
by this line
safe_dict[k] = getLambda(k)

where

def getLambda(k):
    return lambda *a: decimal.Decimal(getattr(math, k)(*a))

According to my understanding, this has something to do with variable referencing in python.
All lambda functions created under that loop referred to the same variable k, so as the value of k change every iteration, all previous created lambda functions is affected. So essentially that loop results in a bunch of copies of the last lambda function. By wrapping it by another function, the variable is forced to be copied.

Hope this helps someone.

@wolph
Copy link
Owner

wolph commented Oct 11, 2020

Good catch, that's indeed an annoying bug. For some reason even with your fix I get weird issues. When I run log(10) it calls lgamma(10) internally and I'm not sure why...

I'm working on fixing the bug in any case, thanks for the help!

@wolph
Copy link
Owner

wolph commented Oct 11, 2020

I found why the tests didn't trip. Several of the methods are natively supported by the decimal class or have custom implementations so they work around the regular implementations. Basically... it only failed when running a math function that had no decimal specific function.

In any case, I've fixed the issues in the new release :)

@wolph wolph closed this as completed in 625065f Oct 11, 2020
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

2 participants