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

built on windows #2

Merged
merged 1 commit into from
Nov 16, 2014
Merged

built on windows #2

merged 1 commit into from
Nov 16, 2014

Conversation

thirdwing
Copy link
Contributor

The mmap for Windows implementation is from https://code.google.com/p/mman-win32/.

And now RcppAnnoy passed the R CMD check on windows (http://win-builder.r-project.org/XXJ1PZFHgu7l/00check.log).

I have used this mmap implementation when porting software to Windows. It works well although the efficiency may not be that good.

@eddelbuettel
Copy link
Owner

You rock. That was unbelievably fast as I only blogged about the wouldn't it be nice this morning!

Now, the patch is [mostly] against annoylib -- so do you want to fork Annoy as well to get a contributor credit? If you'd rather not I'd be happy too as @erikbern has let a few of my PRs in.

But this is very much your work, so you ought to get a credit too ...

eddelbuettel added a commit that referenced this pull request Nov 16, 2014
@eddelbuettel eddelbuettel merged commit 9742f69 into eddelbuettel:master Nov 16, 2014
eddelbuettel added a commit that referenced this pull request Nov 16, 2014
@thirdwing
Copy link
Contributor Author

OK. I will do that myself.

Actually I have been porting some C++ code to Windows for last week and mmap is one of important parts. That is why it can be done so fast.

@eddelbuettel
Copy link
Owner

Sounds good. Tell @erikbern I said Hi :)

@eddelbuettel
Copy link
Owner

Oh, and maybe just copy over the two lines I added in RcppAnnoy to give provenance and license.

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