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

Log #52

Closed
wants to merge 2 commits into from
Closed

Log #52

wants to merge 2 commits into from

Conversation

fponticelli
Copy link
Contributor

I added the log function to the set of math functions. It seems to work fine in some cases (egg below) and fail on other (glass). I am not sure my change introduced those errors or how to protect from them.

(define (glass)
(lambda-shape (x y z)
  (+ (* x x) (* y y) (* (log (+ z 3.2)) (log (+ z 3.2))) 0.02))
)


(define (egg)
(lambda-shape (x y z)
  (+ (* x x) (* y y) (* (log (+ z 3.2)) z) 0.02))
)

@mkeeter
Copy link
Member

mkeeter commented Oct 19, 2017

Thanks, I'll check this out and see why glass isn't working.

@mkeeter
Copy link
Member

mkeeter commented Oct 28, 2017

Are you sure that you're using the chain rule correctly for the derivatives?

For example, in the derivative evaluator, you're saying

od = ad * log(av);

but checking with Wolfram Alpha shows
screen shot 2017-10-28 at 9 15 07 am

mkeeter added a commit that referenced this pull request Nov 10, 2017
@mkeeter
Copy link
Member

mkeeter commented Nov 10, 2017

Yup, the derivatives are noticeably wrong. Look at the bottom of the egg here:
screen shot 2017-11-10 at 9 37 53 am

vs with fixed derivatives:
screen shot 2017-11-10 at 9 37 18 am

I fixed the derivatives, rebased, and merged this in e6d8c67

(glass is still crashing, but that's a kernel bug and I'll open up another issue for that)

@mkeeter mkeeter closed this Nov 10, 2017
@mkeeter mkeeter mentioned this pull request Nov 10, 2017
@fponticelli
Copy link
Contributor Author

That's great, thank you!

@fponticelli fponticelli deleted the log branch November 11, 2017 00:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants