fix(tf): fix compatibility with TF 2.20 #4890
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Jinzhe Zeng <jinzhe.zeng@ustc.edu.cn>
Signed-off-by: Jinzhe Zeng <jinzhe.zeng@ustc.edu.cn>
Signed-off-by: Jinzhe Zeng <jinzhe.zeng@ustc.edu.cn>
Signed-off-by: Jinzhe Zeng <jinzhe.zeng@ustc.edu.cn>
Signed-off-by: Jinzhe Zeng <jinzhe.zeng@ustc.edu.cn>
Signed-off-by: Jinzhe Zeng <jinzhe.zeng@ustc.edu.cn>
Signed-off-by: Jinzhe Zeng <jinzhe.zeng@ustc.edu.cn>
Signed-off-by: Jinzhe Zeng <jinzhe.zeng@ustc.edu.cn>
Signed-off-by: Jinzhe Zeng <jinzhe.zeng@ustc.edu.cn>
Signed-off-by: Jinzhe Zeng <jinzhe.zeng@ustc.edu.cn>
This reverts commit 335ed72.
This reverts commit 4862beb.
This reverts commit 5752d67.
This reverts commit c492696.
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR fixes compatibility issues with TensorFlow 2.20 by addressing version detection and API changes. The main purpose is to ensure the project continues to work with newer TensorFlow versions while maintaining backward compatibility.
- Updates version detection logic in both CMake and Python to handle TF 2.20's removed version macros
- Switches from deprecated
TF_VERSION_STRINGtoTF_Version()C API function - Pins TensorFlow to <2.20 on Windows due to symbol export limitations
- Fixes TENSORFLOW_ROOT environment variable setting across CI and development scripts
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| source/cmake/tf_version.cpp | Switches from deprecated TF_VERSION_STRING to TF_Version() C API |
| source/cmake/Findtensorflow.cmake | Adds manual version macro definitions for TF 2.20+ and links required libraries |
| backend/find_tensorflow.py | Adds fallback version detection from setup.py and error handling for invalid TENSORFLOW_ROOT |
| .github/workflows/*.yml | Fixes TENSORFLOW_ROOT environment variable by adding missing importlib.util import |
| .devcontainer/* | Fixes TENSORFLOW_ROOT environment variable in development scripts |
Comments suppressed due to low confidence (1)
source/cmake/Findtensorflow.cmake:311
- There's a typo in the comment: 'manuanlly' should be 'manually'.
# TF_PATCH_VERSION are not defined We manuanlly define them in our CMake files
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Jinzhe Zeng <njzjz@qq.com>
Contributor
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughPython one-liners in devcontainer scripts and CI now import importlib.util for TensorFlow location. Backend TensorFlow discovery adds stricter error handling, Windows wheel constraint, and a TF ≥2.20 version fallback. CMake TF detection links TF libs, captures stdout, and defines TF_* version macros; helper binary uses TF_Version(). Changes
Sequence Diagram(s)sequenceDiagram
participant CI/Build
participant FindTensorFlow.cmake
participant tf_version (helper)
participant TensorFlow Libs
CI/Build->>FindTensorFlow.cmake: configure
FindTensorFlow.cmake->>tf_version (helper): try_run (link with TF libs)
tf_version (helper)->>TensorFlow Libs: TF_Version()
TensorFlow Libs-->>tf_version (helper): "2.x.y[+suffix]"
tf_version (helper)-->>FindTensorFlow.cmake: stdout version string
alt TF ≥ 2.20
FindTensorFlow.cmake->>FindTensorFlow.cmake: parse major/minor/patch
FindTensorFlow.cmake->>CI/Build: define TF_* macros
else TF < 2.20
FindTensorFlow.cmake->>CI/Build: existing flow
end
sequenceDiagram
participant Caller
participant find_tensorflow.py
participant Python import system
participant Filesystem
Caller->>find_tensorflow.py: find_tensorflow()/get_tf_version()
alt TENSORFLOW_ROOT set
find_tensorflow.py->>Python import system: find_spec("tensorflow") under root
alt spec not found
find_tensorflow.py-->>Caller: raise RuntimeError
else spec found
find_tensorflow.py-->>Caller: root path
end
else no env override
find_tensorflow.py->>Python import system: find_spec("tensorflow")
find_tensorflow.py-->>Caller: resolved root
end
Caller->>find_tensorflow.py: get_tf_version()
find_tensorflow.py->>Filesystem: read version.h
alt version.h missing (TF ≥ 2.20)
find_tensorflow.py->>Filesystem: read tools/pip_package/setup.py
find_tensorflow.py->>find_tensorflow.py: regex parse _VERSION
alt parse failed
find_tensorflow.py-->>Caller: raise RuntimeError
else parsed
find_tensorflow.py-->>Caller: major.minor.patch[+suffix]
end
else got version.h
find_tensorflow.py-->>Caller: major.minor.patch[+suffix]
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~35 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these settings in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (6)
.devcontainer/gdb_pytest_lmp (1)
5-5: Make TENSORFLOW_ROOT discovery resilient to namespace packages and None originfind_spec("tensorflow").origin can be None for namespace packages. Prefer submodule_search_locations when available; fall back to origin.parent otherwise.
Apply this diff:
-TENSORFLOW_ROOT=$(python -c 'import importlib.util,pathlib;print(pathlib.Path(importlib.util.find_spec("tensorflow").origin).parent)') +TENSORFLOW_ROOT=$(python -c 'import importlib.util, pathlib; s=importlib.util.find_spec("tensorflow"); print(s.submodule_search_locations[0] if s and s.submodule_search_locations else pathlib.Path(s.origin).parent)').github/workflows/test_cuda.yml (1)
49-49: Harden TENSORFLOW_ROOT derivation to handle namespace packagesSame rationale as other scripts: use submodule_search_locations when present to avoid None origin edge-cases.
Apply this diff:
- export TENSORFLOW_ROOT=$(python -c 'import importlib.util,pathlib;print(pathlib.Path(importlib.util.find_spec("tensorflow").origin).parent)') + export TENSORFLOW_ROOT=$(python -c 'import importlib.util, pathlib; s=importlib.util.find_spec("tensorflow"); print(s.submodule_search_locations[0] if s and s.submodule_search_locations else pathlib.Path(s.origin).parent)').github/workflows/test_python.yml (1)
30-30: Use package search locations when resolving TENSORFLOW_ROOTAlign with robust resolution: prefer submodule_search_locations, fall back to origin.parent.
Apply this diff:
- export TENSORFLOW_ROOT=$(python -c 'import importlib.util,pathlib;print(pathlib.Path(importlib.util.find_spec("tensorflow").origin).parent)') + export TENSORFLOW_ROOT=$(python -c 'import importlib.util, pathlib; s=importlib.util.find_spec("tensorflow"); print(s.submodule_search_locations[0] if s and s.submodule_search_locations else pathlib.Path(s.origin).parent)')source/cmake/tf_version.cpp (2)
12-12: Print a trailing newline to avoid parsing/IO edge-casesSome consumers expect a newline-terminated line; also helps ensure the output is flushed immediately.
Apply this diff:
- std::cout << TF_Version(); + std::cout << TF_Version() << std::endl;
10-12: Comment wording nit: avoid time-stamped phrasing“Aug 2025” will age quickly. Prefer neutral phrasing.
Apply this diff:
- // Aug 2025: since TF 2.20, TF_VERSION_STRING is no more available; - // try to use the C API TF_Version + // Since TF 2.20, TF_VERSION_STRING is no longer available; + // use the C API TF_Version() instead.source/cmake/Findtensorflow.cmake (1)
309-324: TF ≥ 2.20 macro definitions: good approach; tighten polish (typo, modernize defines, guard parse failure)The parsing/definition strategy looks sound. Two improvements:
- Fix the typo in the comment ("manuanlly" → "manually") and punctuation.
- Prefer add_compile_definitions over add_definitions for clarity and modern CMake.
- Emit a warning when the regex does not match, so consumers know macros were not defined.
Proposed diff:
if(TENSORFLOW_VERSION VERSION_GREATER_EQUAL 2.20) # since TF 2.20, macros like TF_MAJOR_VERSION, TF_MINOR_VERSION, and - # TF_PATCH_VERSION are not defined We manuanlly define them in our CMake files + # TF_PATCH_VERSION are not defined. We manually define them in CMake. # firstly, split TENSORFLOW_VERSION (e.g. 2.20.0rc0) to 2 20 0 rc0 string(REGEX MATCH "^([0-9]+)\\.([0-9]+)\\.([0-9]+)(.*)$" _match ${TENSORFLOW_VERSION}) if(_match) set(TF_MAJOR_VERSION ${CMAKE_MATCH_1}) set(TF_MINOR_VERSION ${CMAKE_MATCH_2}) set(TF_PATCH_VERSION ${CMAKE_MATCH_3}) # add defines - add_definitions(-DTF_MAJOR_VERSION=${TF_MAJOR_VERSION}) - add_definitions(-DTF_MINOR_VERSION=${TF_MINOR_VERSION}) - add_definitions(-DTF_PATCH_VERSION=${TF_PATCH_VERSION}) + add_compile_definitions( + TF_MAJOR_VERSION=${TF_MAJOR_VERSION} + TF_MINOR_VERSION=${TF_MINOR_VERSION} + TF_PATCH_VERSION=${TF_PATCH_VERSION} + ) + else() + message(WARNING "Failed to parse TENSORFLOW_VERSION='${TENSORFLOW_VERSION}'. TF_* version macros will not be defined.") endif() endif()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (11)
.devcontainer/build_cxx.sh(1 hunks).devcontainer/gdb_lmp(1 hunks).devcontainer/gdb_pytest_lmp(1 hunks).devcontainer/lmp(1 hunks).devcontainer/pytest_lmp(1 hunks).github/workflows/test_cc.yml(1 hunks).github/workflows/test_cuda.yml(1 hunks).github/workflows/test_python.yml(1 hunks)backend/find_tensorflow.py(4 hunks)source/cmake/Findtensorflow.cmake(2 hunks)source/cmake/tf_version.cpp(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (29)
- GitHub Check: Build C++ (cpu, cpu)
- GitHub Check: Test Python (6, 3.12)
- GitHub Check: Test Python (5, 3.12)
- GitHub Check: Test Python (1, 3.12)
- GitHub Check: Build C++ (clang, clang)
- GitHub Check: Build C++ (rocm, rocm)
- GitHub Check: Build C++ (cuda120, cuda)
- GitHub Check: Build C++ (cuda, cuda)
- GitHub Check: Build wheels for cp311-macosx_arm64
- GitHub Check: Test Python (6, 3.9)
- GitHub Check: Build C library (2.18, libdeepmd_c.tar.gz)
- GitHub Check: Build C library (2.14, >=2.5.0,<2.15, libdeepmd_c_cu11.tar.gz)
- GitHub Check: Build wheels for cp311-macosx_x86_64
- GitHub Check: Build wheels for cp311-manylinux_x86_64
- GitHub Check: Test C++ (true)
- GitHub Check: Build wheels for cp310-manylinux_aarch64
- GitHub Check: Test Python (3, 3.12)
- GitHub Check: Build wheels for cp311-manylinux_x86_64
- GitHub Check: Analyze (python)
- GitHub Check: Build wheels for cp311-win_amd64
- GitHub Check: Test Python (4, 3.12)
- GitHub Check: Test Python (3, 3.9)
- GitHub Check: Test Python (4, 3.9)
- GitHub Check: Test Python (5, 3.9)
- GitHub Check: Test Python (1, 3.9)
- GitHub Check: Analyze (c-cpp)
- GitHub Check: Test C++ (false)
- GitHub Check: Test Python (2, 3.12)
- GitHub Check: Test Python (2, 3.9)
🔇 Additional comments (2)
backend/find_tensorflow.py (2)
60-63: Good: hard-fail early when TENSORFLOW_ROOT is invalidRaising a RuntimeError if no TensorFlow is discoverable under TENSORFLOW_ROOT makes misconfiguration fail fast and aligns with the CI logic.
161-163: Approve: Windows CPU pin to <2.20 is correctVerified that the specifier "!=2.15.,<2.20" excludes 2.15. and all 2.20+ releases while allowing older 2.x releases.
- File to note:
- backend/find_tensorflow.py — lines 161-163
Snippet:
# Since TF 2.20, not all symbols are exported to the public API. "tensorflow-cpu!=2.15.*,<2.20; platform_system=='Windows'", # https://github.com/h5py/h5py/issues/2408Verification results (from the provided check): {'2.14.1': True, '2.15.0': False, '2.15.1': False, '2.19.0': True, '2.20.0': False, '2.20.1': False}
iProzd
approved these changes
Aug 15, 2025
wanghan-iapcm
approved these changes
Aug 15, 2025
github-merge-queue bot
pushed a commit
that referenced
this pull request
Oct 2, 2025
## Summary This PR bumps the CMake minimum required version from 3.16 to 3.25.2 across the repository to support the `RUN_OUTPUT_STDOUT_VARIABLE` option used in `source/cmake/Findtensorflow.cmake`. ## Background PR #4890 introduced the use of `RUN_OUTPUT_STDOUT_VARIABLE` in the `try_run()` command within `source/cmake/Findtensorflow.cmake` (line 297). This CMake feature was added in CMake 3.25, but the minimum required version was still set to 3.16, which would cause build failures on systems with older CMake versions. Additionally, CMake 3.25.0 and 3.25.1 contain a bug (https://gitlab.kitware.com/cmake/cmake/-/issues/24275), so the minimum version is set to 3.25.2 to avoid this issue. ## Changes This PR updates the CMake version requirements consistently across all build configuration files: - **Main CMakeLists.txt**: Updated base requirement and conditional requirements for CUDA/ROCM variants - **Python build configuration** (`backend/read_env.py`): Updated version requirements for CPU, CUDA, and ROCM variants - **Documentation** (`doc/install/install-from-source.md`): Simplified to state that CMake 3.25.2+ is required for all platforms - **Test and example CMakeLists**: Updated all test directories and example projects to require CMake 3.25.2 for consistency ### Files Modified - `source/CMakeLists.txt` - `backend/read_env.py` - `doc/install/install-from-source.md` - `source/lib/src/gpu/CMakeLists.txt` - `source/lib/tests/CMakeLists.txt` - `source/api_c/tests/CMakeLists.txt` - `source/api_cc/tests/CMakeLists.txt` - `source/lmp/plugin/CMakeLists.txt` - `examples/infer_water/CMakeLists.txt` ## Impact Users will need CMake 3.25.2 or later to build DeePMD-kit from source. CMake 3.25.2 was released in March 2022, so it should be widely available. Users can easily install or upgrade CMake via pip: ```bash pip install -U cmake ``` Fixes the issue where builds would fail with CMake versions between 3.16 and 3.24 due to the use of `RUN_OUTPUT_STDOUT_VARIABLE`, and avoids the bug present in CMake 3.25.0 and 3.25.1. <!-- START COPILOT CODING AGENT SUFFIX --> <details> <summary>Original prompt</summary> > > ---- > > *This section details on the original issue you should resolve* > > <issue_title>[BUG] CMake minimal version should be bumped to 3.25</issue_title> > <issue_description>### Bug summary > > #4890 uses `RUN_OUTPUT_STDOUT_VARIABLE` which is new in CMake 3.25. Thus, the CMake minimal version should be bumped to 3.25 in CMakeLists.txt, read_env.py, and the documentation. > > ### DeePMD-kit Version > > devel > > ### Backend and its version > > - > > ### How did you download the software? > > Built from source > > ### Input Files, Running Commands, Error Log, etc. > > As above. > > ### Steps to Reproduce > > As above. > > ### Further Information, Files, and Links > > _No response_</issue_description> > > ## Comments on the Issue (you are @copilot in this section) > > <comments> > </comments> > </details> Fixes #5000 <!-- START COPILOT CODING AGENT TIPS --> --- 💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click [here](https://survey3.medallia.com/?EAHeSx-AP01bZqG0Ld9QLQ) to start the survey. --------- Signed-off-by: Jinzhe Zeng <jinzhe.zeng@ustc.edu.cn> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: njzjz <9496702+njzjz@users.noreply.github.com> Co-authored-by: Jinzhe Zeng <jinzhe.zeng@ustc.edu.cn>
ChiahsinChu
pushed a commit
to ChiahsinChu/deepmd-kit
that referenced
this pull request
Dec 17, 2025
Fix version finding in pip and CMake; pin TF to <2.20 on Windows; fix TENSORFLOW_ROOT in the CI. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - New Features - Added compatibility with TensorFlow 2.20+ via runtime version detection and generated version macros. - Bug Fixes - Clearer errors when a specified TensorFlow root is invalid. - Improved version-parsing fallback for newer TensorFlow releases. - Tightened Windows CPU wheel constraint to avoid incompatible versions. - Chores - Updated devcontainer scripts and CI workflows to more reliably locate TensorFlow without importing it directly. - Linked TensorFlow during version checks to ensure accurate detection. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Jinzhe Zeng <jinzhe.zeng@ustc.edu.cn> Signed-off-by: Jinzhe Zeng <njzjz@qq.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
ChiahsinChu
pushed a commit
to ChiahsinChu/deepmd-kit
that referenced
this pull request
Dec 17, 2025
## Summary This PR bumps the CMake minimum required version from 3.16 to 3.25.2 across the repository to support the `RUN_OUTPUT_STDOUT_VARIABLE` option used in `source/cmake/Findtensorflow.cmake`. ## Background PR deepmodeling#4890 introduced the use of `RUN_OUTPUT_STDOUT_VARIABLE` in the `try_run()` command within `source/cmake/Findtensorflow.cmake` (line 297). This CMake feature was added in CMake 3.25, but the minimum required version was still set to 3.16, which would cause build failures on systems with older CMake versions. Additionally, CMake 3.25.0 and 3.25.1 contain a bug (https://gitlab.kitware.com/cmake/cmake/-/issues/24275), so the minimum version is set to 3.25.2 to avoid this issue. ## Changes This PR updates the CMake version requirements consistently across all build configuration files: - **Main CMakeLists.txt**: Updated base requirement and conditional requirements for CUDA/ROCM variants - **Python build configuration** (`backend/read_env.py`): Updated version requirements for CPU, CUDA, and ROCM variants - **Documentation** (`doc/install/install-from-source.md`): Simplified to state that CMake 3.25.2+ is required for all platforms - **Test and example CMakeLists**: Updated all test directories and example projects to require CMake 3.25.2 for consistency ### Files Modified - `source/CMakeLists.txt` - `backend/read_env.py` - `doc/install/install-from-source.md` - `source/lib/src/gpu/CMakeLists.txt` - `source/lib/tests/CMakeLists.txt` - `source/api_c/tests/CMakeLists.txt` - `source/api_cc/tests/CMakeLists.txt` - `source/lmp/plugin/CMakeLists.txt` - `examples/infer_water/CMakeLists.txt` ## Impact Users will need CMake 3.25.2 or later to build DeePMD-kit from source. CMake 3.25.2 was released in March 2022, so it should be widely available. Users can easily install or upgrade CMake via pip: ```bash pip install -U cmake ``` Fixes the issue where builds would fail with CMake versions between 3.16 and 3.24 due to the use of `RUN_OUTPUT_STDOUT_VARIABLE`, and avoids the bug present in CMake 3.25.0 and 3.25.1. <!-- START COPILOT CODING AGENT SUFFIX --> <details> <summary>Original prompt</summary> > > ---- > > *This section details on the original issue you should resolve* > > <issue_title>[BUG] CMake minimal version should be bumped to 3.25</issue_title> > <issue_description>### Bug summary > > deepmodeling#4890 uses `RUN_OUTPUT_STDOUT_VARIABLE` which is new in CMake 3.25. Thus, the CMake minimal version should be bumped to 3.25 in CMakeLists.txt, read_env.py, and the documentation. > > ### DeePMD-kit Version > > devel > > ### Backend and its version > > - > > ### How did you download the software? > > Built from source > > ### Input Files, Running Commands, Error Log, etc. > > As above. > > ### Steps to Reproduce > > As above. > > ### Further Information, Files, and Links > > _No response_</issue_description> > > ## Comments on the Issue (you are @copilot in this section) > > <comments> > </comments> > </details> Fixes deepmodeling#5000 <!-- START COPILOT CODING AGENT TIPS --> --- 💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click [here](https://survey3.medallia.com/?EAHeSx-AP01bZqG0Ld9QLQ) to start the survey. --------- Signed-off-by: Jinzhe Zeng <jinzhe.zeng@ustc.edu.cn> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: njzjz <9496702+njzjz@users.noreply.github.com> Co-authored-by: Jinzhe Zeng <jinzhe.zeng@ustc.edu.cn>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix version finding in pip and CMake; pin TF to <2.20 on Windows; fix TENSORFLOW_ROOT in the CI.
Summary by CodeRabbit
New Features
Bug Fixes
Chores