Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

cleaner code, lower costs, and higher performance #1

Merged
merged 1 commit into from Jul 25, 2014

Conversation

Projects
None yet
3 participants
Contributor

renkun-ken commented Jul 24, 2014

Use R's convention of vectorization.
Use Map() to iterate over multiple vectors without replicating them.

Owner

sjp commented Jul 25, 2014

Thanks.

I'll merge this.

Have you experienced material performance improvement from this change? I have just timed some test runs between the CRAN version and the package with your changes and have noticed no real change.

It is nice to have cleaner code though :)

@sjp sjp added a commit that referenced this pull request Jul 25, 2014

@sjp sjp Merge pull request #1 from renkun-ken/master
cleaner code, lower costs, and higher performance
379a628

@sjp sjp merged commit 379a628 into sjp:master Jul 25, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
Contributor

renkun-ken commented Jul 25, 2014

The performance improvement appears only when the arguments like selector take long vectors and the function is intensively called. However, it's very rare condition.

Owner

sjp commented Jul 25, 2014

Yep, as I suspected. The cleaner code alone is worth it.

Thanks again.

Owner

renkun-ken commented on R/main.R in bf67573 Jul 25, 2014

It seems that this line is unnecessary too, neither is 1L:n. Map will handle this automatically.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment