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

Improve handling of git info in version number #348

Merged
merged 4 commits into from
Dec 4, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
* text=auto
featomic/include/featomic.h eol=lf
4 changes: 3 additions & 1 deletion .github/workflows/build-wheels.yml
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,12 @@ jobs:
CIBW_BUILD: cp310-*
CIBW_SKIP: "*musllinux*"
CIBW_ARCHS: ${{ matrix.cibw_arch }}
CIBW_BUILD_VERBOSITY: 2
CIBW_BUILD_VERBOSITY: 1
CIBW_MANYLINUX_X86_64_IMAGE: rustc-manylinux2014_x86_64
CIBW_REPAIR_WHEEL_COMMAND_MACOS: "delocate-wheel --ignore-missing-dependencies --require-archs {delocate_archs} -w {dest_dir} -v {wheel}"
CIBW_REPAIR_WHEEL_COMMAND_LINUX: "auditwheel repair --exclude libmetatensor.so -w {dest_dir} {wheel}"
CIBW_ENVIRONMENT: >
MACOSX_DEPLOYMENT_TARGET=11

- uses: actions/upload-artifact@v3
with:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/coverage.yml
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ jobs:

- name: combine Python coverage files
run: |
coverage combine --append ./python/featomic/.coverage ./python/featomic-torch/.coverage
coverage combine --append ./python/featomic/.coverage ./python/featomic_torch/.coverage
coverage xml

- name: upload to codecov.io
Expand Down
6 changes: 6 additions & 0 deletions .github/workflows/rust-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,12 @@ jobs:
run: sudo rm -rf /usr/share/dotnet /usr/local/lib/android || true

- uses: actions/checkout@v4
with:
fetch-depth: 0

- name: Configure git safe directory
if: matrix.container == 'ubuntu:20.04'
run: git config --global --add safe.directory /__w/featomic/featomic

- name: "copy the code to C: drive"
if: matrix.os == 'windows-2019'
Expand Down
23 changes: 23 additions & 0 deletions .github/workflows/torch-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ jobs:
tests:
runs-on: ${{ matrix.os }}
name: ${{ matrix.os }} / Torch ${{ matrix.torch-version }}
container: ${{ matrix.container }}
strategy:
matrix:
include:
Expand All @@ -28,6 +29,14 @@ jobs:
cargo-test-flags: --release
do-valgrind: true

- os: ubuntu-20.04
container: ubuntu:20.04
extra-name: ", cmake 3.16"
torch-version: 2.5.*
python-version: "3.12"
cargo-test-flags: ""
cxx-flags: -fsanitize=undefined -fsanitize=address -fno-omit-frame-pointer -g

- os: macos-14
torch-version: 2.3.*
python-version: "3.12"
Expand All @@ -40,7 +49,20 @@ jobs:
cargo-test-flags: --release

steps:
- name: install dependencies in container
if: matrix.container == 'ubuntu:20.04'
run: |
apt update
apt install -y software-properties-common
apt install -y cmake make gcc g++ git curl

- uses: actions/checkout@v4
with:
fetch-depth: 0

- name: Configure git safe directory
if: matrix.container == 'ubuntu:20.04'
run: git config --global --add safe.directory /__w/featomic/featomic

- name: setup rust
uses: dtolnay/rust-toolchain@master
Expand Down Expand Up @@ -77,3 +99,4 @@ jobs:
# Use the CPU only version of torch when building/running the code
PIP_EXTRA_INDEX_URL: https://download.pytorch.org/whl/cpu
FEATOMIC_TORCH_TEST_VERSION: ${{ matrix.torch-version }}
CXXFLAGS: ${{ matrix.cxx-flags }}
2 changes: 1 addition & 1 deletion .readthedocs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ build:
# install featomic-torch with the CPU version of PyTorch. We can not use
# the `python` section below since it does not allow to specify
# `--extra-index-url`
- pip install --extra-index-url https://download.pytorch.org/whl/cpu python/featomic-torch
- pip install --extra-index-url https://download.pytorch.org/whl/cpu python/featomic_torch
pre_build:
# Pre-build Rust code here to avoid timeout when building docs
- cargo build
Expand Down
2 changes: 1 addition & 1 deletion CONTRIBUTING.rst
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ browser
tox
coverage combine --append \
./.coverage \
./python/featomic-torch/.coverage
./python/featomic_torch/.coverage
coverage html
firefox htmlcov/index.html

Expand Down
4 changes: 2 additions & 2 deletions docs/src/get-started/installation.rst
Original file line number Diff line number Diff line change
Expand Up @@ -106,14 +106,14 @@ Building from source:
.. code-block:: bash

git clone https://github.com/metatensor/featomic
cd featomic/python/featomic-torch
cd featomic/python/featomic_torch
pip install .

# Make sure you are using the latest version of pip
pip install --upgrade pip

# alternatively, the same thing in a single command
pip install git+https://github.com/metatensor/featomic#subdirectory=python/featomic-torch
pip install git+https://github.com/metatensor/featomic#subdirectory=python/featomic_torch


For usage from C++
Expand Down
69 changes: 64 additions & 5 deletions featomic-torch/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,18 @@ endif()
file(READ ${CMAKE_CURRENT_SOURCE_DIR}/VERSION FEATOMIC_TORCH_VERSION)
string(STRIP ${FEATOMIC_TORCH_VERSION} FEATOMIC_TORCH_VERSION)

include(cmake/dev-versions.cmake)
create_development_version("${FEATOMIC_TORCH_VERSION}" FEATOMIC_TORCH_FULL_VERSION)
message(STATUS "Building featomic-torch v${FEATOMIC_TORCH_FULL_VERSION}")

# strip any -dev/-rc suffix on the version since project(VERSION) does not support it
string(REGEX REPLACE "([0-9]*)\\.([0-9]*)\\.([0-9]*).*" "\\1.\\2.\\3" FEATOMIC_TORCH_VERSION ${FEATOMIC_TORCH_FULL_VERSION})
project(featomic_torch
VERSION ${FEATOMIC_TORCH_VERSION}
LANGUAGES CXX
)
set(PROJECT_VERSION ${FEATOMIC_TORCH_FULL_VERSION})


option(FEATOMIC_TORCH_TESTS "Build featomic-torch C++ tests" OFF)
option(FEATOMIC_TORCH_FETCH_METATENSOR_TORCH "Download and build the metatensor_torch library before building featomic_torch" OFF)
Expand All @@ -46,8 +54,56 @@ if (${CMAKE_CURRENT_SOURCE_DIR} STREQUAL ${CMAKE_SOURCE_DIR})
endif()
endif()

set(REQUIRED_FEATOMIC_VERSION "0.1")
find_package(featomic ${REQUIRED_FEATOMIC_VERSION} CONFIG REQUIRED)
function(check_compatible_versions _dependency_ _actual_ _requested_)
if(${_actual_} MATCHES "^([0-9]+)\\.([0-9]+)")
set(_actual_major_ "${CMAKE_MATCH_1}")
set(_actual_minor_ "${CMAKE_MATCH_2}")
else()
message(FATAL_ERROR "Failed to parse actual version: ${_actual_}")
endif()

if(${_requested_} MATCHES "^([0-9]+)\\.([0-9]+)")
set(_requested_major_ "${CMAKE_MATCH_1}")
set(_requested_minor_ "${CMAKE_MATCH_2}")
else()
message(FATAL_ERROR "Failed to parse requested version: ${_requested_}")
endif()

if (${_requested_major_} EQUAL 0 AND ${_actual_minor_} EQUAL ${_requested_minor_})
# major version is 0 and same minor version, everything is fine
elseif (${_actual_major_} EQUAL ${_requested_major_})
# same major version, everything is fine
else()
# not compatible
message(FATAL_ERROR "Incompatible versions for ${_dependency_}: we need v${_requested_}, but we got v${_actual_}")
endif()
endfunction()


set(REQUIRED_FEATOMIC_VERSION "0.0.0")
if (NOT "$ENV{FEATOMIC_NO_LOCAL_DEPS}" STREQUAL "1")
# If building a dev version, we also need to update the
# REQUIRED_FEATOMIC_VERSION in the same way we update the
# featomic-torch version
create_development_version("${REQUIRED_FEATOMIC_VERSION}" FEATOMIC_FULL_VERSION)
else()
set(FEATOMIC_FULL_VERSION ${REQUIRED_FEATOMIC_VERSION})
endif()
string(REGEX REPLACE "([0-9]*)\\.([0-9]*).*" "\\1.\\2" REQUIRED_FEATOMIC_VERSION ${FEATOMIC_FULL_VERSION})

# Either featomic is built as part of the same CMake project, or we try to
# find the corresponding CMake package
if (TARGET featomic)
get_target_property(FEATOMIC_BUILD_VERSION featomic BUILD_VERSION)
check_compatible_versions("featomic" ${FEATOMIC_BUILD_VERSION} ${REQUIRED_FEATOMIC_VERSION})
else()
find_package(featomic ${REQUIRED_FEATOMIC_VERSION} CONFIG REQUIRED)

get_target_property(FEATOMIC_LOCATION featomic IMPORTED_LOCATION)
get_filename_component(FEATOMIC_LOCATION ${FEATOMIC_LOCATION} DIRECTORY)
message(STATUS "Using local featomic from ${FEATOMIC_LOCATION}")
endif()


# FindCUDNN.cmake distributed with PyTorch is a bit broken, so we have a
# fixed version in `cmake/FindCUDNN.cmake`
Expand All @@ -58,17 +114,17 @@ find_package(Torch 1.12 REQUIRED)
# ============================================================================ #
# Setup metatensor_torch

set(METATENSOR_FETCH_VERSION "0.6.0")
set(METATENSOR_FETCH_VERSION "0.6.1")
set(REQUIRED_METATENSOR_TORCH_VERSION "0.6")
if (FEATOMIC_TORCH_FETCH_METATENSOR_TORCH)
message(STATUS "Fetching metatensor-torch @ ${METATENSOR_FETCH_VERSION} from github")
message(STATUS "Fetching metatensor-torch from github")

set(URL_ROOT "https://github.com/lab-cosmo/metatensor/releases/download")
include(FetchContent)
FetchContent_Declare(
metatensor_torch
URL ${URL_ROOT}/metatensor-torch-v${METATENSOR_FETCH_VERSION}/metatensor-torch-cxx-${METATENSOR_FETCH_VERSION}.tar.gz
URL_HASH SHA256=f050743662ece38948b2087dd025d60110645716840dbfc5370c059e1275d0cf
URL_HASH SHA256=0941da4bc6d25ee73b597774d3c8c6edf6a44f134139bd93b33834eae52ac4dd
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should maybe document somewhere what to do when lifting the metatensor version.

Including updating the hashes etc...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, if you update the version number above and not this, then CMake will yell at you. But yeah, we should add documentation around this!

Copy link
Member Author

@Luthaf Luthaf Dec 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added some comments in the CMakeLists.txt, should be good for now!

)

if (CMAKE_VERSION VERSION_GREATER 3.18)
Expand All @@ -82,6 +138,9 @@ if (FEATOMIC_TORCH_FETCH_METATENSOR_TORCH)
endif()
else()
find_package(metatensor_torch ${REQUIRED_METATENSOR_TORCH_VERSION} REQUIRED CONFIG)
get_target_property(METATENSOR_TORCH_LOCATION metatensor_torch IMPORTED_LOCATION)
get_filename_component(METATENSOR_TORCH_LOCATION ${METATENSOR_TORCH_LOCATION} DIRECTORY)
message(STATUS "Using local metatensor-torch from ${METATENSOR_TORCH_LOCATION}")
endif()


Expand Down
2 changes: 1 addition & 1 deletion featomic-torch/VERSION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
0.1.0
0.0.0
96 changes: 96 additions & 0 deletions featomic-torch/cmake/dev-versions.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
# Parse a `_version_` number, and store its components in `_major_` `_minor_`
# `_patch_` and `_rc_`
function(parse_version _version_ _major_ _minor_ _patch_ _rc_)
string(REGEX MATCH "([0-9]+)\\.([0-9]+)\\.([0-9]+)(-rc)?([0-9]+)?" _ "${_version_}")

if(${CMAKE_MATCH_COUNT} EQUAL 3)
set(${_rc_} "" PARENT_SCOPE)
elseif(${CMAKE_MATCH_COUNT} EQUAL 5)
set(${_rc_} ${CMAKE_MATCH_5} PARENT_SCOPE)
else()
message(FATAL_ERROR "invalid version string ${_version_}")
endif()

set(${_major_} ${CMAKE_MATCH_1} PARENT_SCOPE)
set(${_minor_} ${CMAKE_MATCH_2} PARENT_SCOPE)
set(${_patch_} ${CMAKE_MATCH_3} PARENT_SCOPE)
endfunction()

if (CMAKE_VERSION VERSION_LESS "3.17")
# CMAKE_CURRENT_FUNCTION_LIST_DIR was added in CMake 3.17
set(CMAKE_CURRENT_FUNCTION_LIST_DIR "${CMAKE_CURRENT_LIST_DIR}")
endif()

# Get the time of the last modification since the last tag/release, and a hash
# of the latest commit/full state of a dirty repository
function(git_version_info _output_n_commits_ _output_git_hash_)
set(_script_ "${CMAKE_CURRENT_FUNCTION_LIST_DIR}/../../scripts/git-version-info.py")

if (EXISTS "${CMAKE_CURRENT_FUNCTION_LIST_DIR}/git_version_info")
# When building from a tarball, the script is executed and the result
# put in this file
file(STRINGS "${CMAKE_CURRENT_FUNCTION_LIST_DIR}/git_version_info" _file_content_)
list(GET _file_content_ 0 _n_commits_)
list(GET _file_content_ 1 _git_hash_)

elseif (EXISTS "${_script_}")
# When building from a checkout, we'll need to run the script
find_package(Python COMPONENTS Interpreter REQUIRED)
execute_process(
COMMAND "${Python_EXECUTABLE}" "${_script_}" "featomic-torch-v"
RESULT_VARIABLE _status_
OUTPUT_VARIABLE _stdout_
ERROR_VARIABLE _stderr_
WORKING_DIRECTORY ${CMAKE_CURRENT_FUNCTION_LIST_DIR}
)

if (NOT ${_status_} EQUAL 0)
message(WARNING
"git-version-info.py failed, version number might be wrong:\nstdout: ${_stdout_}\nstderr: ${_stderr_}")
set(${_output_} 0 PARENT_SCOPE)
return()
endif()

if (NOT "${_stderr_}" STREQUAL "")
message(WARNING "git-version-info.py gave some errors, version number might be wrong:\nstdout: ${_stdout_}\nstderr: ${_stderr_}")
endif()

string(REPLACE "\n" ";" _lines_ ${_stdout_})
list(GET _lines_ 0 _n_commits_)
list(GET _lines_ 1 _git_hash_)
else()
message(FATAL_ERROR "could not update git version information")
endif()

string(STRIP ${_n_commits_} _n_commits_)
set(${_output_n_commits_} ${_n_commits_} PARENT_SCOPE)

string(STRIP ${_git_hash_} _git_hash_)
set(${_output_git_hash_} ${_git_hash_} PARENT_SCOPE)
endfunction()


# Take the version declared in the package, and increase the right number if we
# are actually installing a developement version from after the latest git tag
function(create_development_version _version_ _output_)
git_version_info(_n_commits_ _git_hash_)

parse_version(${_version_} _major_ _minor_ _patch_ _rc_)
if(${_n_commits_} STREQUAL "0")
# we are building a release, leave the version number as-is
if("${_rc_}" STREQUAL "")
set(${_output_} "${_major_}.${_minor_}.${_patch_}" PARENT_SCOPE)
else()
set(${_output_} "${_major_}.${_minor_}.${_patch_}-rc${_rc_}" PARENT_SCOPE)
endif()
else()
# we are building a development version, increase the right part of the version
if("${_rc_}" STREQUAL "")
math(EXPR _minor_ "${_minor_} + 1")
set(${_output_} "${_major_}.${_minor_}.0-dev${_n_commits_}+${_git_hash_}" PARENT_SCOPE)
else()
math(EXPR _rc_ "${_rc_} + 1")
set(${_output_} "${_major_}.${_minor_}.${_patch_}-rc${_rc_}-dev${_n_commits_}+${_git_hash_}" PARENT_SCOPE)
endif()
endif()
endfunction()
9 changes: 8 additions & 1 deletion featomic-torch/tests/cmake-project/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,14 @@ cmake_minimum_required(VERSION 3.16)

project(featomic-torch-test-cmake-project CXX)

find_package(featomic_torch 0.1 CONFIG REQUIRED)

# We need to update the REQUIRED_FEATOMIC_VERSION in the same way we update the
# featomic version for dev builds
include(../../cmake/dev-versions.cmake)
set(REQUIRED_FEATOMIC_TORCH_VERSION "0.0.0")
create_development_version("${REQUIRED_FEATOMIC_TORCH_VERSION}" FEATOMIC_TORCH_FULL_VERSION)
string(REGEX REPLACE "([0-9]*)\\.([0-9]*).*" "\\1.\\2" REQUIRED_FEATOMIC_TORCH_VERSION ${FEATOMIC_TORCH_FULL_VERSION})
find_package(featomic_torch ${REQUIRED_FEATOMIC_TORCH_VERSION} REQUIRED)

add_executable(torch-main src/main.cpp)
target_link_libraries(torch-main featomic_torch)
Expand Down
Loading
Loading