You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
This repository has been archived by the owner on Mar 19, 2024. It is now read-only.
The 'Requirements' section of the documentation says:
_Generally, fastText builds on modern Mac OS and Linux distributions. Since it uses some C++11 features, it requires a compiler with good C++11 support. These include :
(g++-4.7.2 or newer) or (clang-3.3 or newer)_
Recent performance improvements use C++17 features (specifically the use of std::string_view). Reference issue - Predict 1.9-4.2x faster (#1341)
Action
Although C++17 is now a prerequisite, fastText can still compile on C++11 with a few minor modifications, but with degraded performance. To do this:
Dependency on string_view is restored using a type definition on C++11.
Fix build errors caused by use of static_assert
Add instructions in the README file for modifications to makefile to build on C++11
The feature is desirable because legacy systems that use fastText (since 2015) might continue to use C++11.
Recommendation
Add string_view to dictionary for fast lookup (commit ffee8e4) incorrectly replaces a pass by reference by a pass by value on several constant argument. Use a pass by reference instead.
The text was updated successfully, but these errors were encountered:
* Add string_view to dictionary for fast lookup (commit [ffee8e4](https://github.com/facebookresearch/fastText/commit/ffee8e4d72a4d2ecd859575007877d12acbee5b3)) incorrectly replaces a pass by reference by a pass by value on several constant argument. Use a pass by reference instead.
I can see why the argument about pass by value in the article is strong. The implicit assumption is that you have a 64-bit processor that the compiler can use to pass by value. It is fair, because most modern computers are 64-bit at the very least.
I concur with the argument. The exception is if the implementation of string_view changes to something more substantial than what a qwprd can hold, then it is a problem.
What would you like me to do? I am happy to leave it as is, given the article. Shall we just document it and leave the code as is?
Regards,Sreekant
On Thursday, February 1, 2024 at 02:10:50 a.m. GMT+1, Kenneth Heafield ***@***.***> wrote:
Can you convert this to a pull request?
* Add string_view to dictionary for fast lookup (commit [ffee8e4](ffee8e4)) incorrectly replaces a pass by reference by a pass by value on several constant argument. Use a pass by reference instead.
https://quuxplusone.github.io/blog/2021/11/09/pass-string-view-by-value/
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you authored the thread.Message ID: ***@***.***>
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Issue
The 'Requirements' section of the documentation says:
_Generally, fastText builds on modern Mac OS and Linux distributions. Since it uses some C++11 features, it requires a compiler with good C++11 support. These include :
(g++-4.7.2 or newer) or (clang-3.3 or newer)_
Recent performance improvements use C++17 features (specifically the use of std::string_view). Reference issue - Predict 1.9-4.2x faster (#1341)
Action
Although C++17 is now a prerequisite, fastText can still compile on C++11 with a few minor modifications, but with degraded performance. To do this:
The feature is desirable because legacy systems that use fastText (since 2015) might continue to use C++11.
Recommendation
The text was updated successfully, but these errors were encountered: