Skip to content

Commit

Permalink
Remove featomic::systems::read_from_file
Browse files Browse the repository at this point in the history
This allow to disable the chemfiles dependency by default for the rust crate.

This makes examples in C and C++ a bit more complex, but also a lot closer
to how someone would actually use the code from these languages.
  • Loading branch information
Luthaf committed Nov 25, 2024
1 parent 5388ad8 commit 8d0b9d7
Show file tree
Hide file tree
Showing 41 changed files with 505 additions and 544 deletions.
6 changes: 5 additions & 1 deletion .github/workflows/rust-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@ jobs:
run: cargo test --lib --target ${{ matrix.rust-target }} ${{ matrix.cargo-build-flags }}

- name: documentation tests
# we need rustc 1.78 to load libmetatensor.so in doctests
if: matrix.rust-version == 'stable'
run: cargo test --doc --target ${{ matrix.rust-target }} ${{ matrix.cargo-build-flags }}

- name: integration tests
Expand Down Expand Up @@ -148,7 +150,9 @@ jobs:
echo "CMAKE_CXX_COMPILER_LAUNCHER=sccache" >> $GITHUB_ENV
- name: check that examples compile & run
run: cargo run --release --example compute-soap -- featomic/examples/data/water.xyz
run: |
cargo run --release --features chemfiles --example compute-soap -- featomic/examples/data/water.xyz
cargo run --release --features chemfiles --example profiling -- featomic/examples/data/water.xyz
- name: check that benchmarks compile and run once
run: cargo bench -- --test
Expand Down
2 changes: 1 addition & 1 deletion MANIFEST.in
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ prune python/*.egg-info

prune featomic/tests
prune featomic/benches/data
prune featomic/examples
prune featomic/examples/data

prune featomic-c-api/tests
prune featomic-c-api/examples
Expand Down
11 changes: 0 additions & 11 deletions docs/src/get-started/installation.rst
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,6 @@ The build and installation can be configures with a few cmake options, using
| FEATOMIC_INSTALL_BOTH_STATIC_SHARED | Install both the shared and static version | ON |
| | of the library | |
+--------------------------------------+-----------------------------------------------+----------------+
| FEATOMIC_ENABLE_CHEMFILES | Enable the usage of chemfiles for reading | ON |
| | systems from files | |
+--------------------------------------+-----------------------------------------------+----------------+
| FEATOMIC_FETCH_METATENSOR | Automatically fetch, build and install | OFF |
| | metatensor (a dependency of featomic) | |
+--------------------------------------+-----------------------------------------------+----------------+
Expand All @@ -95,14 +92,6 @@ Add the following to your project ``Cargo.toml``
[dependencies]
featomic = {git = "https://github.com/metatensor/featomic"}
Featomic has one optional dependency (chemfiles), which is enabled by default.
If you want to disable it, you can use:

.. code-block:: toml
[dependencies]
featomic = {git = "https://github.com/metatensor/featomic", default-features = false}
.. _install-torch-script:

Expand Down
2 changes: 0 additions & 2 deletions docs/src/references/api/c/misc.rst
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ Error handling

.. doxygendefine:: FEATOMIC_UTF8_ERROR

.. doxygendefine:: FEATOMIC_CHEMFILES_ERROR

.. doxygendefine:: FEATOMIC_SYSTEM_ERROR

.. doxygendefine:: FEATOMIC_INTERNAL_ERROR
Expand Down
6 changes: 0 additions & 6 deletions docs/src/references/api/c/systems.rst
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,3 @@ all required functions; and then passing one or more systems to

.. doxygenstruct:: featomic_pair_t
:members:

---------------------------------------------------------------------

.. doxygenfunction:: featomic_basic_systems_read

.. doxygenfunction:: featomic_basic_systems_free
9 changes: 1 addition & 8 deletions docs/src/references/api/cxx/systems.rst
Original file line number Diff line number Diff line change
@@ -1,16 +1,9 @@
Defining systems
================

There are two ways you can define systems to pass to a
:cpp:class:`featomic::Calculator`. The easy way is to use
:cpp:class:`featomic::BasicSystems` to read all systems defined in a file, and
run the calculation on all these systems. The more complex but also more
flexible way is to define a new child class of :cpp:class:`featomic::System`
implementing all required functions; and then passing a vector of pointers to
the child class instances to your :cpp:class:`featomic::Calculator`.

.. doxygenclass:: featomic::System
:members:

.. doxygenclass:: featomic::BasicSystems
.. doxygenclass:: featomic::SimpleSystem
:members:
14 changes: 1 addition & 13 deletions featomic-c-api/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@ if (POLICY CMP0135)
endif()

if (POLICY CMP0077)
# use variables to set OPTIONS
cmake_policy(SET CMP0077 NEW)
cmake_policy(SET CMP0077 NEW) # use variables to set OPTIONS
endif()

file(STRINGS "Cargo.toml" CARGO_TOML_CONTENT)
Expand Down Expand Up @@ -60,7 +59,6 @@ set(RUST_BUILD_TARGET "" CACHE STRING "Cross-compilation target for rust code. L
set(EXTRA_RUST_FLAGS "" CACHE STRING "Flags used to build rust code")
mark_as_advanced(RUST_BUILD_TARGET EXTRA_RUST_FLAGS)

option(FEATOMIC_ENABLE_CHEMFILES "Enable the usage of chemfiles for reading structures from files" OFF)
option(FEATOMIC_FETCH_METATENSOR "Download and build the metatensor C API before building featomic" OFF)

set(CMAKE_MACOSX_RPATH ON)
Expand Down Expand Up @@ -158,10 +156,6 @@ else()
set(CARGO_OUTPUT_DIR "${CARGO_TARGET_DIR}/${RUST_BUILD_TARGET}/${CARGO_BUILD_TYPE}")
endif()

if (NOT ${FEATOMIC_ENABLE_CHEMFILES})
set(CARGO_BUILD_ARG "${CARGO_BUILD_ARG};--no-default-features")
endif()

# Get the list of libraries linked by default by cargo/rustc to add when linking
# to featomic::static
if (CARGO_VERSION_CHANGED)
Expand Down Expand Up @@ -331,9 +325,6 @@ set(FEATOMIC_INCLUDE_DIR ${PROJECT_SOURCE_DIR}/include/)
set_target_properties(featomic::shared PROPERTIES
IMPORTED_LOCATION ${FEATOMIC_SHARED_LOCATION}
INTERFACE_INCLUDE_DIRECTORIES ${FEATOMIC_INCLUDE_DIR}
# the library will need to be linked as C++ code
# since it might contains chemfiles
IMPORTED_LINK_INTERFACE_LANGUAGES CXX
)
target_compile_features(featomic::shared INTERFACE cxx_std_17)

Expand All @@ -347,9 +338,6 @@ set_target_properties(featomic::static PROPERTIES
IMPORTED_LOCATION ${FEATOMIC_STATIC_LOCATION}
INTERFACE_INCLUDE_DIRECTORIES ${FEATOMIC_INCLUDE_DIR}
INTERFACE_LINK_LIBRARIES "${CARGO_DEFAULT_LIBRARIES}"
# the library will need to be linked as C++ code
# since it might contains chemfiles
IMPORTED_LINK_INTERFACE_LANGUAGES CXX
)

if (BUILD_SHARED_LIBS)
Expand Down
5 changes: 1 addition & 4 deletions featomic-c-api/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ version = "0.1.0"
authors = ["Guillaume Fraux <[email protected]>"]
edition = "2021"
rust-version = "1.74"
publish = false

[lib]
# when https://github.com/rust-lang/cargo/pull/8789 lands, use it here!
Expand All @@ -12,10 +13,6 @@ name = "featomic"
crate-type = ["cdylib", "staticlib"]
bench = false

[features]
default = ["chemfiles"]
chemfiles = ["featomic/chemfiles"]

[dependencies]
featomic = {path = "../featomic", version = "0.1.0", default-features = false}
metatensor = "0.2"
Expand Down
4 changes: 4 additions & 0 deletions featomic-c-api/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,8 @@ fn main() {
} else {
// if featomic header generation failed, we always re-run the build script
}

println!(
"cargo:rustc-env=TARGET={}", std::env::var("TARGET").expect("missing TARGET env variable")
);
}
2 changes: 0 additions & 2 deletions featomic-c-api/cmake/featomic-config.in.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ if (@FEATOMIC_INSTALL_BOTH_STATIC_SHARED@ OR @BUILD_SHARED_LIBS@)
set_target_properties(featomic::shared PROPERTIES
IMPORTED_LOCATION ${FEATOMIC_SHARED_LOCATION}
INTERFACE_INCLUDE_DIRECTORIES ${FEATOMIC_INCLUDE}
IMPORTED_LINK_INTERFACE_LANGUAGES CXX
)
target_link_libraries(featomic::shared INTERFACE metatensor::shared)

Expand Down Expand Up @@ -65,7 +64,6 @@ if (@FEATOMIC_INSTALL_BOTH_STATIC_SHARED@ OR NOT @BUILD_SHARED_LIBS@)
IMPORTED_LOCATION ${FEATOMIC_STATIC_LOCATION}
INTERFACE_INCLUDE_DIRECTORIES ${FEATOMIC_INCLUDE}
INTERFACE_LINK_LIBRARIES "@CARGO_DEFAULT_LIBRARIES@"
IMPORTED_LINK_INTERFACE_LANGUAGES CXX
)
target_link_libraries(featomic::static INTERFACE metatensor::shared)

Expand Down
186 changes: 186 additions & 0 deletions featomic-c-api/examples/common/systems.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,186 @@
#include <stdio.h>

#include <chemfiles.h>

#include "systems.h"


// This file shows an example on how to define a custom `featomic_system_t`,
// based on chemfiles (https://chemfiles.org).
//
// The core idea is to define a set of functions to communicate data (atomic
// types, positions, periodic cell, …) between your code and featomic. Each
// function takes a `void* user_data` parameter, which you can set to anything
// relevant.

typedef struct {
uint64_t size;
CHFL_FRAME* frame;
int32_t* atom_types;
} chemfiles_system_t;

static featomic_status_t chemfiles_system_size(const void* system, uintptr_t* size) {
*size = ((const chemfiles_system_t*)system)->size;
return FEATOMIC_SUCCESS;
}

static featomic_status_t chemfiles_system_cell(const void* system, double cell[9]) {
CHFL_CELL* chemfiles_cell = chfl_cell_from_frame(((const chemfiles_system_t*)system)->frame);
if (chemfiles_cell == NULL) {
printf("Error: %s", chfl_last_error());
return -1;
}

chfl_status status = chfl_cell_matrix(chemfiles_cell, (chfl_vector3d*)cell);
chfl_free(chemfiles_cell);

if (status == CHFL_SUCCESS) {
return FEATOMIC_SUCCESS;
} else {
printf("Error: %s", chfl_last_error());
return -2;
}
}

static featomic_status_t chemfiles_system_positions(const void* system, const double** positions) {
chfl_vector3d* chemfiles_positions = NULL;
uint64_t size = 0;
chfl_status status = chfl_frame_positions(
((const chemfiles_system_t*)system)->frame,
&chemfiles_positions,
&size
);

if (status == CHFL_SUCCESS) {
*positions = (const double*)chemfiles_positions;
return FEATOMIC_SUCCESS;
} else {
return -1;
}
}

static featomic_status_t chemfiles_system_types(const void* system, const int32_t** types) {
*types = ((const chemfiles_system_t*)system)->atom_types;
return FEATOMIC_SUCCESS;
}

static featomic_status_t chemfiles_system_neighbors(void* system, double cutoff) {
// this system does not have a neighbor list, and needs to use the one
// inside featomic with `use_native_system=true`
return -1;
}


int read_systems_example(const char* path, featomic_system_t** systems, uintptr_t* n_systems) {
CHFL_TRAJECTORY* trajectory = NULL;
CHFL_ATOM* atom = NULL;
chemfiles_system_t* system = NULL;
chfl_status status;
uint64_t step = 0;
uint64_t n_steps = 0;

trajectory = chfl_trajectory_open(path, 'r');
if (trajectory == NULL) {
printf("Error: %s", chfl_last_error());
goto error;
}

status = chfl_trajectory_nsteps(trajectory, &n_steps);
if (status != CHFL_SUCCESS) {
printf("Error: %s", chfl_last_error());
goto error;
}

*systems = calloc(n_steps, sizeof(featomic_system_t));
if (*systems == NULL) {
printf("Error: Failed to allocate systems");
goto error;
}
*n_systems = (uintptr_t)n_steps;

for (step=0; step<n_steps; step++) {
system = calloc(1, sizeof(chemfiles_system_t));
if (system == NULL) {
printf("Error: failed to allocate single system");
goto error;
}

system->frame = chfl_frame();
if (system->frame == NULL) {
printf("Error: %s", chfl_last_error());
goto error;
}

status = chfl_trajectory_read_step(trajectory, step, system->frame);
if (status != CHFL_SUCCESS) {
printf("Error: %s", chfl_last_error());
goto error;
}

// extract atomic types from the frame
chfl_frame_atoms_count(system->frame, &system->size);
if (status != CHFL_SUCCESS) {
printf("Error: %s", chfl_last_error());
goto error;
}

system->atom_types = calloc(system->size, sizeof(int32_t));
if (system->atom_types == NULL) {
printf("Error: failed to allocate atom_types");
goto error;
}

for (uint64_t i=0; i<system->size; i++) {
atom = chfl_atom_from_frame(system->frame, i);
if (atom == NULL) {
printf("Error: %s", chfl_last_error());
goto error;
}

uint64_t number = 0;
chfl_atom_atomic_number(atom, &number);
system->atom_types[i] = (int32_t)(number);

chfl_free(atom);
}

// setup all the data and functions for the system
(*systems)[step].user_data = system;

(*systems)[step].size = chemfiles_system_size;
(*systems)[step].cell = chemfiles_system_cell;
(*systems)[step].positions = chemfiles_system_positions;
(*systems)[step].types = chemfiles_system_types;
(*systems)[step].compute_neighbors = chemfiles_system_neighbors;
(*systems)[step].pairs = NULL;
(*systems)[step].pairs_containing = NULL;

system = NULL;
}

chfl_free(trajectory);
return 0;

error:
chfl_free(trajectory);
chfl_free(atom);

// cleanup any sucesfully allocated frames
free_systems_example(*systems, step);

*systems = NULL;
*n_systems = 0;

return 1;
}

void free_systems_example(featomic_system_t* systems, uintptr_t n_systems) {
for (size_t i=0; i<n_systems; i++) {
chemfiles_system_t* system = systems[i].user_data;
chfl_free(system->frame);
free(system->atom_types);
free(system);
}

free(systems);
}
9 changes: 9 additions & 0 deletions featomic-c-api/examples/common/systems.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#ifndef FEATOMIC_EXAMPLE_SYSTEMS_H
#define FEATOMIC_EXAMPLE_SYSTEMS_H

#include <featomic.h>

int read_systems_example(const char* path, featomic_system_t** systems, uintptr_t* n_systems);
void free_systems_example(featomic_system_t* systems, uintptr_t n_systems);

#endif
Loading

0 comments on commit 8d0b9d7

Please sign in to comment.