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

[r2] Collected changes and fixes from devel #3224

Merged
merged 6 commits into from
Feb 4, 2024

Conversation

njzjz
Copy link
Member

@njzjz njzjz commented Feb 4, 2024

Cherry-pick: #3163, #3181, #3217, #3221, #3223, #3206

Note

This PR should not be squashed.

pre-commit-ci bot and others added 6 commits February 3, 2024 12:51
<!--pre-commit.ci start-->
updates:
- [github.com/astral-sh/ruff-pre-commit: v0.1.13 →
v0.1.14](astral-sh/ruff-pre-commit@v0.1.13...v0.1.14)
<!--pre-commit.ci end-->

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
(cherry picked from commit bc00a0a)
Fix the following compiler warning:
```
/home/runner/work/deepmd-kit/deepmd-kit/source/api_c/src/c_api.cc:1336:17: warning: returning address of local temporary object [-Wreturn-stack-address]
  return (int*)&(dcm->dcm.sel_types())[0];
                ^~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
```
by returning the reference of `sel_type`.

`DataChargeModifier.sel_types` is not used anywhere, even in the test,
so we don't have a chance to determine if there is a possible segfault,
and this warning has no actual impact.

It seems `DeepTensor` has returned a reference since the beginning
(deepmodeling#137). (perhaps because
`DeepTensor.sel_types` is used) `DeepTensor` and `DataChargeModifier`
have different returned types.

(cherry picked from commit f900f3a)
Fix deepmodeling#3214.

In the gmx patch file, `${TENSORFLOW_ROOT}` is used other than
`${TensorFlow_LIBRARY_PATH}$` or `${TENSORFLOW_INCLUDE_DIRS}`, so the
fastest workaround is to set `${TENSORFLOW_ROOT}`.

https://github.com/deepmodeling/deepmd-kit/blob/eb9b2efedf4efc946894800a0d7abf5056f4bb7a/source/gmx/patches/2020.2/CMakeLists.txt.patch.in#L14-L18

Signed-off-by: Jinzhe Zeng <[email protected]>
(cherry picked from commit 701b913)
Fix deepmodeling#3214.

Signed-off-by: Jinzhe Zeng <[email protected]>
(cherry picked from commit 412c812)
Fix deepmodeling#3045. All memory leaks have been fixed!

---------

Signed-off-by: Jinzhe Zeng <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
(cherry picked from commit ab2c551)
Today [GitHub introduced the new M1
runners](https://github.blog/changelog/2024-01-30-github-actions-introducing-the-new-m1-macos-runner-available-to-open-source/),
making it possible to build macos-arm64 wheels without cross-building.

Remove old hacked codes for cross-building.

(cherry picked from commit 664c70b)
Copy link

codecov bot commented Feb 4, 2024

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (b875ea8) 76.06% compared to head (fc868a9) 76.21%.

Files Patch % Lines
source/api_c/include/deepmd.hpp 90.90% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##               r2    #3224      +/-   ##
==========================================
+ Coverage   76.06%   76.21%   +0.15%     
==========================================
  Files         277      277              
  Lines       25603    25639      +36     
  Branches     1591     1605      +14     
==========================================
+ Hits        19476    19542      +66     
- Misses       5203     5225      +22     
+ Partials      924      872      -52     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@njzjz njzjz changed the title [r2] Collected fixes from devel [r2] Collected changes and fixes from devel Feb 4, 2024
@wanghan-iapcm wanghan-iapcm merged commit be43748 into deepmodeling:r2 Feb 4, 2024
47 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants