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

[BUG] CMake incorrectly forces C++14 #4569

Closed
oschuett opened this issue Jan 23, 2025 · 2 comments · Fixed by #4570
Closed

[BUG] CMake incorrectly forces C++14 #4569

oschuett opened this issue Jan 23, 2025 · 2 comments · Fixed by #4570
Assignees
Labels

Comments

@oschuett
Copy link

oschuett commented Jan 23, 2025

Bug summary

I was unable to build DeePMD with PyTorch from source because the code requires C++17 but CMake forces C++14:

if(NOT DEEPMD_C_ROOT)
# we can still allow C++ 11 for programs linked to the C library
set_if_higher(CMAKE_CXX_STANDARD 14)
endif()

DeePMD-kit Version

3.0.1

Backend and its version

PyTorch

How did you download the software?

Built from source

Input Files, Running Commands, Error Log, etc.

Building DeePMD with PyTorch like this:

cmake -DENABLE_PYTORCH=TRUE -DCMAKE_CXX_STANDARD=17 -DCMAKE_CXX_STANDARD_REQUIRED=TRUE ..
make -j deepmd_c

Leads to errors like these:

/home/ole/git/cp2k/tools/toolchain/build/deepmd-kit-3.0.1/source/api_cc/src/DeepSpinPT.cc: In member function ‘void deepmd::DeepSpinPT::compute(ENERGYVTYPE&, std::vector<VALUETYPE>&, std::vector<VALUETYPE>&, std::vector<VALUETYPE>&, std::vector<VALUETYPE>&, std::vector<VALUETYPE>&, const std::vector<VALUETYPE>&, const std::vector<VALUETYPE>&, const std::vector<int>&, const std::vector<VALUETYPE>&, int, const deepmd::InputNlist&, const int&, const std::vector<VALUETYPE>&, const std::vector<VALUETYPE>&, bool)’:
/home/ole/git/cp2k/tools/toolchain/build/deepmd-kit-3.0.1/source/api_cc/src/DeepSpinPT.cc:148:12: error: ‘is_same_v’ is not a member of ‘std’; did you mean ‘is_same’?
  148 |   if (std::is_same_v<VALUETYPE, float>) {
      |            ^~~~~~~~~
      |            is_same
/home/ole/git/cp2k/tools/toolchain/build/deepmd-kit-3.0.1/source/api_cc/src/DeepSpinPT.cc:148:31: error: expected primary-expression before ‘,’ token
  148 |   if (std::is_same_v<VALUETYPE, float>) {
      |                               ^
/home/ole/git/cp2k/tools/toolchain/build/deepmd-kit-3.0.1/source/api_cc/src/DeepSpinPT.cc:148:33: error: expected primary-expression before ‘float’
  148 |   if (std::is_same_v<VALUETYPE, float>) {
      |                                 ^~~~~
/home/ole/git/cp2k/tools/toolchain/build/deepmd-kit-3.0.1/source/api_cc/src/DeepSpinPT.cc:148:32: error: expected ‘)’ before ‘float’
  148 |   if (std::is_same_v<VALUETYPE, float>) {
      |      ~                         ^~~~~~
      |                                )

Steps to Reproduce

cmake -DENABLE_PYTORCH=TRUE -DCMAKE_CXX_STANDARD=17 -DCMAKE_CXX_STANDARD_REQUIRED=TRUE ..
make -j deepmd_c

Further Information, Files, and Links

No response

@njzjz
Copy link
Member

njzjz commented Jan 24, 2025

Thanks for report. I didn't realize that C++ 17 has been used.

njzjz added a commit to njzjz/deepmd-kit that referenced this issue Jan 24, 2025
@njzjz njzjz linked a pull request Jan 24, 2025 that will close this issue
github-merge-queue bot pushed a commit that referenced this issue Jan 24, 2025
Fix #4569.

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- **Build Configuration**
- Enhanced flexibility in C++ standard settings based on project
dependencies
	- Added conditional checks for setting C++ standard version

- **Code Improvements**
	- Updated type checking mechanisms in compute methods
- Refined method signatures for handling different data types in
DeepPotPT and DeepSpinPT classes

- **Technical Refinements**
	- Improved conditional compilation and configuration handling
	- Introduced more flexible type-specific method overloads

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
@njzjz njzjz closed this as completed Jan 25, 2025
@oschuett
Copy link
Author

Thanks a lot for the quick fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants