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

Issue with the exp layer when the base is e #3937

Merged
merged 1 commit into from
Apr 8, 2016
Merged

Conversation

emaggiori
Copy link

This fixes a bug in the exp layer. In the case of the base e (set by the default parameter base==-1), the outer_scale_ variable was computed by taking -1 as a base instead of e.

The new code uses exp(input_shift) instead of pow(base, input_shift) when the base is set to -1.

@jeffdonahue
Copy link
Contributor

Thanks @emaggiori, this does seem like a bug, yet I did write tests for what I think should be this exact case, and those are clearly passing. So I'm not sure what's going on yet...any ideas? Ideally this would include a new test demonstrating the bug and that it's now fixed & passing.

@jeffdonahue
Copy link
Contributor

Ah... I see now -- the bug is only triggered when shift is non-zero, which is not tested. (That makes more sense because I don't think I've ever used that parameter...) Could you add a new ExpLayer test for that case, after the existing ones I linked in the previous comment?

(This is just one more piece of evidence in favor of modularity -- and in particular in favor of splitting off the scale/shift params from Exp and Power layer, as @longjon has argued for.)

@emaggiori
Copy link
Author

Hi, I added the tests. Let me know if this is okay.

@jeffdonahue
Copy link
Contributor

Great, thanks @emaggiori! Please squash your changes to a single commit and I'll merge this.

@emaggiori
Copy link
Author

Thanks @jeffdonahue, I squashed them to a single commit as suggested.

@shelhamer shelhamer added the bug label Apr 8, 2016
@jeffdonahue
Copy link
Contributor

Looks good, thanks again @emaggiori.

@jeffdonahue jeffdonahue merged commit d21772c into BVLC:master Apr 8, 2016
fxbit pushed a commit to Yodigram/caffe that referenced this pull request Sep 1, 2016
Issue with the exp layer when the base is e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants