Skip to content

Commit

Permalink
Apply linter suggestions for C++ and Rust code
Browse files Browse the repository at this point in the history
I used clang-tidy for C++ code, and clippy for Rust code
  • Loading branch information
Luthaf authored and PicoCentauri committed Oct 11, 2023
1 parent 449ac37 commit 434f923
Show file tree
Hide file tree
Showing 18 changed files with 88 additions and 77 deletions.
2 changes: 1 addition & 1 deletion rascaline-c-api/src/calculator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ fn convert_labels_selection<'a>(
}
(true, false) => {
let tensor = unsafe {
TensorMap::from_raw(selection.predefined as *mut mts_tensormap_t)
TensorMap::from_raw(selection.predefined.cast_mut())
};

match tensor.try_clone() {
Expand Down
78 changes: 46 additions & 32 deletions rascaline-c-api/tests/calculator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@
static void check_block(
mts_tensormap_t* descriptor,
size_t block_id,
std::vector<int32_t> samples,
std::vector<int32_t> properties,
std::vector<double> values,
std::vector<int32_t> gradient_samples,
std::vector<double> gradients
const std::vector<int32_t>& samples,
const std::vector<int32_t>& properties,
const std::vector<double>& values,
const std::vector<int32_t>& gradient_samples,
const std::vector<double>& gradients
);

TEST_CASE("calculator name") {
Expand All @@ -26,7 +26,7 @@ TEST_CASE("calculator name") {
auto* calculator = rascal_calculator("dummy_calculator", HYPERS_JSON);
REQUIRE(calculator != nullptr);

char buffer[256] = {0};
char buffer[256] = {};
CHECK_SUCCESS(rascal_calculator_name(calculator, buffer, sizeof(buffer)));
CHECK(buffer == std::string("dummy test calculator with cutoff: 3.5 - delta: 25 - name: bar"));

Expand Down Expand Up @@ -68,7 +68,7 @@ TEST_CASE("calculator parameters") {
auto* calculator = rascal_calculator("dummy_calculator", HYPERS_JSON.c_str());
REQUIRE(calculator != nullptr);

char buffer[256] = {0};
char buffer[256] = {};
CHECK_SUCCESS(rascal_calculator_parameters(calculator, buffer, sizeof(buffer)));
CHECK(buffer == HYPERS_JSON);

Expand Down Expand Up @@ -138,7 +138,9 @@ TEST_CASE("Compute descriptor") {
SECTION("Full compute") {
auto system = simple_system();

rascal_calculation_options_t options = {0};
rascal_calculation_options_t options;
std::memset(&options, 0, sizeof(rascal_calculation_options_t));

const char* gradients_list[] = {"positions"};
options.gradients = gradients_list;
options.gradients_count = 1;
Expand All @@ -151,7 +153,9 @@ TEST_CASE("Compute descriptor") {
);
CHECK_SUCCESS(status);

mts_labels_t keys = {0};
mts_labels_t keys;
std::memset(&keys, 0, sizeof(mts_labels_t));

status = mts_tensormap_keys(descriptor, &keys);
CHECK_SUCCESS(status);

Expand Down Expand Up @@ -219,15 +223,19 @@ TEST_CASE("Compute descriptor") {
"structure", "center"
};

mts_labels_t selected_samples = {0};
mts_labels_t selected_samples;
std::memset(&selected_samples, 0, sizeof(mts_labels_t));

selected_samples.names = selected_sample_names.data();
selected_samples.values = selected_sample_values.data();
selected_samples.count = 2;
selected_samples.size = 2;

auto system = simple_system();

rascal_calculation_options_t options = {0};
rascal_calculation_options_t options;
std::memset(&options, 0, sizeof(rascal_calculation_options_t));

const char* gradients_list[] = {"positions"};
options.gradients = gradients_list;
options.gradients_count = 1;
Expand All @@ -242,7 +250,9 @@ TEST_CASE("Compute descriptor") {

CHECK_SUCCESS(status);

mts_labels_t keys = {0};
mts_labels_t keys;
std::memset(&keys, 0, sizeof(mts_labels_t));

status = mts_tensormap_keys(descriptor, &keys);
CHECK_SUCCESS(status);

Expand Down Expand Up @@ -297,15 +307,19 @@ TEST_CASE("Compute descriptor") {
"index_delta", "x_y_z"
};

mts_labels_t selected_properties = {0};
mts_labels_t selected_properties;
std::memset(&selected_properties, 0, sizeof(mts_labels_t));

selected_properties.names = selected_property_names.data();
selected_properties.values = selected_property_values.data();
selected_properties.count = 1;
selected_properties.size = 2;

auto system = simple_system();

rascal_calculation_options_t options = {0};
rascal_calculation_options_t options;
std::memset(&options, 0, sizeof(rascal_calculation_options_t));

const char* gradients_list[] = {"positions"};
options.gradients = gradients_list;
options.gradients_count = 1;
Expand All @@ -319,7 +333,7 @@ TEST_CASE("Compute descriptor") {
);
CHECK_SUCCESS(status);

mts_labels_t keys = {0};
mts_labels_t keys = {};
status = mts_tensormap_keys(descriptor, &keys);
CHECK_SUCCESS(status);

Expand Down Expand Up @@ -396,13 +410,13 @@ TEST_CASE("Compute descriptor") {

mts_block_t* blocks[2] = {nullptr, nullptr};

mts_labels_t h_samples = {0};
mts_labels_t h_samples = {};
h_samples.size = 2;
h_samples.names = sample_names.data();
h_samples.count = 1;
h_samples.values = h_sample_values.data();

mts_labels_t h_properties = {0};
mts_labels_t h_properties = {};
h_properties.size = 2;
h_properties.names = property_names.data();
h_properties.count = 1;
Expand All @@ -418,13 +432,13 @@ TEST_CASE("Compute descriptor") {
1, 0,
};

mts_labels_t c_samples = {0};
mts_labels_t c_samples = {};
c_samples.size = 2;
c_samples.names = sample_names.data();
c_samples.count = 1;
c_samples.values = c_sample_values.data();

mts_labels_t c_properties = {0};
mts_labels_t c_properties = {};
c_properties.size = 2;
c_properties.names = property_names.data();
c_properties.count = 1;
Expand All @@ -435,17 +449,17 @@ TEST_CASE("Compute descriptor") {
auto keys_names = std::vector<const char*>{"species_center"};
auto keys_values = std::vector<int32_t>{1, 6};

mts_labels_t keys = {0};
mts_labels_t keys = {};
keys.size = 1;
keys.names = keys_names.data();
keys.count = 2;
keys.values = keys_values.data();

auto predefined = mts_tensormap(keys, blocks, 2);
auto* predefined = mts_tensormap(keys, blocks, 2);
REQUIRE(predefined != nullptr);

auto system = simple_system();
rascal_calculation_options_t options = {0};
rascal_calculation_options_t options = {};
const char* gradients_list[] = {"positions"};
options.gradients = gradients_list;
options.gradients_count = 1;
Expand Down Expand Up @@ -520,7 +534,7 @@ TEST_CASE("Compute descriptor") {
// existing one (1) from the default set of keys. We also put the keys
// in a different order than what would be the default (6, 12).

mts_labels_t selected_keys = {0};
mts_labels_t selected_keys = {};
const char* sample_names[] = {"species_center"};
selected_keys.names = sample_names;
selected_keys.size = 1;
Expand All @@ -531,7 +545,7 @@ TEST_CASE("Compute descriptor") {

auto system = simple_system();

rascal_calculation_options_t options = {0};
rascal_calculation_options_t options = {};
const char* gradients_list[] = {"positions"};
options.gradients = gradients_list;
options.gradients_count = 1;
Expand All @@ -546,7 +560,7 @@ TEST_CASE("Compute descriptor") {
);
CHECK_SUCCESS(status);

mts_labels_t keys = {0};
mts_labels_t keys = {};
status = mts_tensormap_keys(descriptor, &keys);
CHECK_SUCCESS(status);

Expand Down Expand Up @@ -596,19 +610,19 @@ TEST_CASE("Compute descriptor") {
void check_block(
mts_tensormap_t* descriptor,
size_t block_id,
std::vector<int32_t> samples,
std::vector<int32_t> properties,
std::vector<double> values,
std::vector<int32_t> gradient_samples,
std::vector<double> gradients
const std::vector<int32_t>& samples,
const std::vector<int32_t>& properties,
const std::vector<double>& values,
const std::vector<int32_t>& gradient_samples,
const std::vector<double>& gradients
) {
mts_block_t* block = nullptr;

auto status = mts_tensormap_block_by_id(descriptor, &block, block_id);
CHECK_SUCCESS(status);

/**************************************************************************/
mts_labels_t labels = {0};
mts_labels_t labels = {};
status = mts_block_labels(block, 0, &labels);
CHECK_SUCCESS(status);

Expand Down Expand Up @@ -639,7 +653,7 @@ void check_block(
mts_labels_free(&labels);

/**************************************************************************/
mts_array_t array = {0};
mts_array_t array = {};
status = mts_block_data(block, &array);
CHECK_SUCCESS(status);

Expand Down
1 change: 0 additions & 1 deletion rascaline-c-api/tests/cmake-project/src/main.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
#include <stdio.h>

#include <rascaline.h>


Expand Down
4 changes: 1 addition & 3 deletions rascaline-c-api/tests/cmake-project/src/main.cpp
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
#include <stdio.h>

#include <rascaline.hpp>


int main() {
try {
auto calculator = rascaline::Calculator(
"dummy_calculator",
"{\"cutoff\": 3.4, \"delta\": -3, \"name\": \"testing\", \"gradients\": true}"
R"({"cutoff": 3.4, "delta": -3, "name": "testing", "gradients": true})"
);
return 0;
} catch (const rascaline::RascalineError& e) {
Expand Down
2 changes: 1 addition & 1 deletion rascaline-c-api/tests/cxx/systems.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ TEST_CASE("basic systems") {

CHECK(systems.count() == 30);

auto system = systems.systems();
auto* system = systems.systems();
uintptr_t size = 0;
system->size(system->user_data, &size);
CHECK(size == 54);
Expand Down
12 changes: 6 additions & 6 deletions rascaline-c-api/tests/helpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
#define SQRT_3 1.73205080756887729352

rascal_system_t simple_system() {
rascal_system_t system = {0};
rascal_system_t system = {};

system.size = [](const void* _, uintptr_t* size) {
*size = 4;
Expand Down Expand Up @@ -104,25 +104,25 @@ rascal_system_t simple_system() {
}

mts_array_t empty_array(std::vector<size_t> array_shape) {
mts_array_t array = {0};
mts_array_t array = {};

array.ptr = new std::vector<size_t>(array_shape);
array.ptr = new std::vector<size_t>(std::move(array_shape));
array.origin = [](const void *array, mts_data_origin_t *origin){
mts_register_data_origin("c-tests-empty-array", origin);
return MTS_SUCCESS;
};
array.shape = [](const void *array, const uintptr_t** shape, uintptr_t* shape_count){
auto array_shape = static_cast<const std::vector<size_t>*>(array);
const auto* array_shape = static_cast<const std::vector<size_t>*>(array);
*shape = array_shape->data();
*shape_count = array_shape->size();
return MTS_SUCCESS;
};
array.destroy = [](void *array){
auto array_shape = static_cast<std::vector<size_t>*>(array);
auto* array_shape = static_cast<std::vector<size_t>*>(array);
delete array_shape;
};
array.copy = [](const void *array, mts_array_t* new_array){
auto array_shape = static_cast<const std::vector<size_t>*>(array);
const auto* array_shape = static_cast<const std::vector<size_t>*>(array);
*new_array = empty_array(*array_shape);
return MTS_SUCCESS;
};
Expand Down
4 changes: 2 additions & 2 deletions rascaline-c-api/tests/logging.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ static void run_calculation(const char* hypers) {
auto* calculator = rascal_calculator("dummy_calculator", hypers);
REQUIRE(calculator != nullptr);
auto system = simple_system();
rascal_calculation_options_t options = {0};
rascal_calculation_options_t options = {};

mts_tensormap_t* descriptor = nullptr;
CHECK_SUCCESS(rascal_calculator_compute(
Expand All @@ -26,7 +26,7 @@ static void run_calculation(const char* hypers) {

TEST_CASE("Logging") {
auto record_log_events = [](int level, const char* message) {
RECORDED_LOG_EVENTS.push_back(std::make_tuple(level, std::string(message)));
RECORDED_LOG_EVENTS.emplace_back(level, std::string(message));
};
CHECK_SUCCESS(rascal_set_logging_callback(record_log_events));

Expand Down
4 changes: 2 additions & 2 deletions rascaline-c-api/tests/systems.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ TEST_CASE("systems errors") {
auto* calculator = rascal_calculator("dummy_calculator", HYPERS_JSON);
REQUIRE(calculator != nullptr);

rascal_system_t system = {0};
rascal_calculation_options_t options = {0};
rascal_system_t system = {};
rascal_calculation_options_t options = {};

mts_tensormap_t* descriptor = nullptr;
auto status = rascal_calculator_compute(
Expand Down
8 changes: 4 additions & 4 deletions rascaline-torch/include/rascaline/torch/system.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class RASCALINE_TORCH_EXPORT SystemHolder final: public rascaline::System, publi
/// SystemHolder can be move assigned
SystemHolder& operator=(SystemHolder&&) = default;

virtual ~SystemHolder() override = default;
~SystemHolder() override = default;

/*========================================================================*/
/* Functions to implement rascaline::System */
Expand All @@ -66,7 +66,7 @@ class RASCALINE_TORCH_EXPORT SystemHolder final: public rascaline::System, publi

/// @private
CellMatrix cell() const override {
auto data = cell_.data_ptr<double>();
auto* data = cell_.data_ptr<double>();
return CellMatrix{{
{{data[0], data[1], data[2]}},
{{data[3], data[4], data[5]}},
Expand Down Expand Up @@ -117,12 +117,12 @@ class RASCALINE_TORCH_EXPORT SystemHolder final: public rascaline::System, publi
}

/// @private implementation of __len__ for TorchScript
int64_t __len__() const {
int64_t len() const {
return species_.size(0);
}

/// @private implementation of __str__ for TorchScript
std::string __str__() const;
std::string str() const;

// TODO: convert from a Dict[str, TorchTensorMap] for the interface with LAMMPS
// static TorchSystem from_metatensor_dict();
Expand Down
Loading

0 comments on commit 434f923

Please sign in to comment.