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

Merge rs_model.cpp into rs_model_impl.hpp #1082

Merged
merged 3 commits into from Aug 25, 2017

Conversation

Projects
None yet
3 participants
@rcurtin
Member

rcurtin commented Aug 7, 2017

This should fix #1078 and #1076. @Muthiang, @rvx86: can you test this on your systems please? I have not been able to reproduce your problem, so I am not 100% sure if it will fix the issue you are each having.

What I've done here is just put the implementation that's currently in rs_model.cpp into rs_model_impl.hpp as inlined functions, because it seems like the compiler sometimes generates duplicate versions of template functions used in rs_model.cpp. Inlining the functions seems likely to fix this.

I also made a few fixes for some warnings, that is unrelated to the issue at hand but still nice to take care of. :)

@zoq

zoq approved these changes Aug 10, 2017

Works for me too, if I have a minute I'll can setup appveyor to build with mingw.

@rvx86

This comment has been minimized.

Show comment
Hide comment
@rvx86

rvx86 Aug 10, 2017

(Since I still have to support Windows Server 2000 as a target (don't laugh), I use the i686-win32-sjlj-rev0 build from mingw-w64-builds, but the default [i686|x86_64]-posix-[dwarf2|seh]-rev2 builds in the MSYS2 package repo will do)

rvx86 commented Aug 10, 2017

(Since I still have to support Windows Server 2000 as a target (don't laugh), I use the i686-win32-sjlj-rev0 build from mingw-w64-builds, but the default [i686|x86_64]-posix-[dwarf2|seh]-rev2 builds in the MSYS2 package repo will do)

@rcurtin rcurtin merged commit 1cf3671 into mlpack:master Aug 25, 2017

3 checks passed

Style Checks Build finished.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment