Skip to content

clip : rename lerp function to avoid conflict #6894

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

Merged
merged 1 commit into from
Apr 25, 2024

Conversation

danbev
Copy link
Collaborator

@danbev danbev commented Apr 25, 2024

This commit renames the lerp (linear interpolation) function in clip.cpp to avoid a conflict with the lerp function in the standard C++ library when using c++20.

The motivation for this change is to enable projects that use c++20 to be able to compile clip.cpp without having to resort to patching it. The lerp function was added to cmath in version C++20 (202002L) and is why this is not causing any issue at the moment as C++11/C++17 is currently used by llama.cpp.

I realize that llama.cpp uses either C++11 (or C++17 in the case for SYCL) but wanted to ask if this would be an acceptable change just the same.

Refs: https://en.cppreference.com/w/cpp/numeric/lerp

@NeoZhangJianyu
Copy link
Collaborator

Which case will use c++ 20?

@ggerganov
Copy link
Member

It's better to just rename the function to clip_lerp for example

Which case will use c++ 20?

See the description - the intent is to support projects who might want to build using C++20

This commit renamesthe lerp (linear interpolation) function in clip.cpp
to avoid a conflict with the lerp function in the <cmath> standard C++
library when using c++20.

The motivation for this change is to enable projects that use c++20 to
be able to compile clip.cpp without having to resort to patching it. The
lerp function was added to cmath in version C++20 (202002L) and is why
this is not causing any issue at the moment as C++11/C++17 is currently
used by llama.cpp.

I realize that llama.cpp uses either C++11 (or C++17 in the case for
SYCL) but wanted to ask if this would be an acceptable change just the
same.

Refs: https://en.cppreference.com/w/cpp/numeric/lerp
Signed-off-by: Daniel Bevenius <daniel.bevenius@gmail.com>
@danbev danbev changed the title clip : add c++20 macro guard to lerp function in clip : rename lerp function to avoid conflict Apr 25, 2024
@ggerganov ggerganov merged commit 4ab99d8 into ggml-org:master Apr 25, 2024
@NeoZhangJianyu
Copy link
Collaborator

OK, got it! Thank you!

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.

3 participants