xexpx, xexpy

[cossio]

Adds functions xexpx(x) and xexpy(x, y), which compute x * exp(x) and x * exp(y), respectively, but taking the "correct" limit when the exponent is infinite.

Close #15 .

[devmotion]

Thanks!

Can you add and test ChainRules derivatives as well?

[cossio]

I added the ChainRules and addressed your comments @devmotion @tpapp . Care for another look?

[tpapp]

As I said in the comments, I am fine with the branch structure as is, so it is up to you. For all other minor changes I agree with @devmotion's comments (marked them by a 👍).

Once this is done, this PR should be ready to merge. Thanks for doing it!

[cossio]

I addressed the comments. Only thing I don't understand is the objection to using Ω / x in the chain rule @devmotion . I'd rather argue it is worth doing this optimization also for xlogx, etc. (in another PR)

@tpapp I think you requested some changes but I can't find them (probably bc I force pushed).

I would like to see a release once this is merged. Should I bump the version here (to 0.3.7)?

[devmotion]

Only thing I don't understand is the objection to using Ω / x in the chain rule @devmotion . I'd rather argue it is worth doing this optimization also for xlogx, etc. (in another PR)

I guess maybe I didn't express myself clearly enough: I'm not against optimizing the forward and reverse mode rules but I think computing Ω / x is the wrong thing to do here. Instead one should implement frule and rrule manually and reuse the result of exp(x) and exp(y) from the computation of the primal.

But I think it's fine to do that in a separate PR and use the unoptimized version initially.

[cossio]

but I think computing Ω / x is the wrong thing to do here

Why?

[devmotion]

Because it is an unnecessary computation that you can avoid by just reusing exp(x)/exp(y), and it requires you to handle cases such as iszero(x). Hence IMO we should rather use the unoptimized version or implement frule/rrule.

[cossio]

I implemented the explicit frule and rrule as suggested by @devmotion . I also went ahead and used your suggested branches in the primal definitions since this looked more consistent with how the chain rules looked when reusing exp(x) etc.

I also bumped the version. Can we tag a release after merging?

Care to take another look? @devmotion @tpapp

[devmotion]

I think it looks good. I made some suggestions for how the code could be made more readable and simpler (in particular the CR derivatives), and how the tests could be extended.

[cossio]

Incorporated most of your comments (with minor modifications) @devmotion

[devmotion]

Thanks for the PR and for addressing all our comments 🙂

I think the PR is ready to be merged and released but I want to wait if @tpapp has any additional comments.

[tpapp]

LGTM. Thanks again!