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

add 'memoize' option to cache kernel computations #6

Merged
merged 1 commit into from Mar 13, 2013

Conversation

harthur
Copy link
Contributor

@harthur harthur commented Mar 12, 2013

This tries to speed up training by caching the kernel computation for each data pair to avoid re-computation. To enable it add memoize: true to the training options.

I tried it on ~1000 vectors of length ~1000 and the speedup was about 10% on both linear and rbf kernels, so not a huge amount.

@karpathy
Copy link
Owner

This looks good! But it's very suspicious that you can only get ~10% improvement in speed. I would really expect the kernel function to be the bottleneck, especially for 1000-D data.

This may be a silly question because I don't have very good intuitions for optimizations in Javascript, but do you think the culprit could be "if (this.kernelResults) {" line? How about a binary class variable flag for whether or not to use the memoized array instead, and simply checking if it's true or false inside kernelResult()?

Or could it be some overhead from a function call in first place? It would be interesting to profile the code and discover what's taking so long. I might find time to look into this over next few days, and will merge changes then.

@harthur
Copy link
Contributor Author

harthur commented Mar 13, 2013

Yeah, I was surprised too. It's hard to tell because each time you run it it's different, but I did a few runs of both. It's possible I did something wrong.

I've optimized a lot of JS code before, and I've found that function call overhead is basically nothing, even in that many iterations. Same for the if check.

Also, this was comparing memoize: true to memoize: false, so both of them went through the function call and if check.

@karpathy
Copy link
Owner

Yeah, I was only concerned about the nature of the if check, it basically has to search the entire class to see if the member "kernelResults" is there every time it's called, but I guess that's negligible. Ok I'll try to run a few experiments for myself too, probably over weekend.

karpathy added a commit that referenced this pull request Mar 13, 2013
add 'memoize' option to cache kernel computations
@karpathy karpathy merged commit b75b712 into karpathy:master Mar 13, 2013
@harthur harthur mentioned this pull request Mar 19, 2013
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

Successfully merging this pull request may close these issues.

None yet

2 participants