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

Feature/vnf skeleton #5

Merged
merged 40 commits into from
Sep 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
bcade21
add cpp files
Laurynas-Jagutis Aug 20, 2024
c8128be
add skeleton files
Laurynas-Jagutis Aug 20, 2024
90b798a
update skeleton
Laurynas-Jagutis Aug 27, 2024
3c1b5c3
empty lines
Laurynas-Jagutis Sep 6, 2024
d676768
address comments
Laurynas-Jagutis Sep 6, 2024
c37a689
add getter/setter
Laurynas-Jagutis Sep 6, 2024
d6a488f
merge main, fix conflict
Laurynas-Jagutis Sep 6, 2024
048ca8a
switch to .hpp file, adjust cmake
Laurynas-Jagutis Sep 10, 2024
f7fc794
reformat, add default values in constructor
Laurynas-Jagutis Sep 10, 2024
153c6b1
delete cmake comments
Laurynas-Jagutis Sep 11, 2024
c220cd4
adjust .hpp file
Laurynas-Jagutis Sep 11, 2024
cb68985
a version that builds
Laurynas-Jagutis Sep 11, 2024
fbe07e6
remove using from global space
Laurynas-Jagutis Sep 11, 2024
da9af31
move the default values to main constructor
Laurynas-Jagutis Sep 11, 2024
3167f8c
fill in cpp file, fix github complaints
Laurynas-Jagutis Sep 11, 2024
4c1d97c
Put power-grid-model under dependency dir.
Jerry-Jinfeng-Guo Sep 12, 2024
c761e6d
forgot to add file
Jerry-Jinfeng-Guo Sep 13, 2024
a1e01d9
trying to put sonar cloud out of its misery
Jerry-Jinfeng-Guo Sep 13, 2024
05b4c0e
added dependency to setup.py; few other minors
Jerry-Jinfeng-Guo Sep 13, 2024
f2d07e2
Merge pull request #7 from PowerGridModel/feature/reorganize-dependency
mgovers Sep 13, 2024
e335f4b
fix clang tidy
Laurynas-Jagutis Sep 16, 2024
39a802f
add test setup
Laurynas-Jagutis Sep 16, 2024
7be16ef
try to fix sonar cloud
Laurynas-Jagutis Sep 16, 2024
e132c93
address some of the sonar cloud issues
Laurynas-Jagutis Sep 16, 2024
62dd06d
adjust formatting
Laurynas-Jagutis Sep 16, 2024
358b2a4
fix typo
Laurynas-Jagutis Sep 16, 2024
cb8d9bb
allow the user's constdataset through c api
Laurynas-Jagutis Sep 17, 2024
0fd2a70
fix up failing checks
Laurynas-Jagutis Sep 17, 2024
4d8fe43
fix ifndef
Laurynas-Jagutis Sep 17, 2024
46961e5
change docstring
Laurynas-Jagutis Sep 17, 2024
213fb5a
add docstrings to c api
Laurynas-Jagutis Sep 17, 2024
13fa19f
change signature
Laurynas-Jagutis Sep 23, 2024
8ba177a
fix clang tidy
Laurynas-Jagutis Sep 23, 2024
27b4ce6
clang tidy v2
Laurynas-Jagutis Sep 23, 2024
78263c6
change object to instance in doc strings
Laurynas-Jagutis Sep 23, 2024
c7b6a49
change signature add constness as suggested
Laurynas-Jagutis Sep 23, 2024
bd52369
add const
Laurynas-Jagutis Sep 23, 2024
d889789
add a todo reminder
Laurynas-Jagutis Sep 23, 2024
5c644aa
adjust todo
Laurynas-Jagutis Sep 23, 2024
102693e
use the ssh protocol, indent, remove the unnecessary semicolons
Laurynas-Jagutis Sep 23, 2024
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
10 changes: 10 additions & 0 deletions .clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,34 @@ Checks: '
-*,
boost-*,
bugprone-*,
-bugprone-easily-swappable-parameters,
figueroa1395 marked this conversation as resolved.
Show resolved Hide resolved
cert-*,
clang-analyzer-*,
concurrency-*,
cppcoreguidelines-*,
-cppcoreguidelines-avoid-c-arrays,
-cppcoreguidelines-owning-memory,
-cppcoreguidelines-avoid-magic-numbers,
-cppcoreguidelines-pro-bounds-pointer-arithmetic,
-cppcoreguidelines-pro-type-reinterpret-cast,
darwin-*,
hiccp-*,
llvm-*,
-llvm-header-guard,
google-*,
-google-build-using-namespace,
-google-explicit-constructor
misc-*,
-misc-non-private-member-variables-in-classes,
modernize-*,
-modernize-avoid-c-arrays,
-modernize-use-trailing-return-type,
-modernize-use-nodiscard,
performance-*,
portability-*,
readability-*,
-readability-identifier-length,
-readability-magic-numbers,
'

WarningsAsErrors: '*'
Expand Down
4 changes: 3 additions & 1 deletion .github/workflows/clang-tidy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,9 @@ jobs:
- uses: actions/checkout@v4
with:
submodules: 'true'

- name: Initialize and update submodules
run: |
git submodule update --init --recursive
- name: Install packages
run: |
sudo apt-get update
Expand Down
4 changes: 3 additions & 1 deletion .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,9 @@ jobs:
- uses: actions/checkout@v4
with:
submodules: 'true'

- name: Initialize and update submodules
run: |
git submodule update --init --recursive
- name: Install packages
run: |
sudo apt-get update
Expand Down
3 changes: 3 additions & 0 deletions .github/workflows/sonar.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ jobs:
with:
fetch-depth: 0 # Shallow clones should be disabled for a better relevancy of analysis
submodules: 'true'
- name: Initialize and update submodules
run: |
git submodule update --init --recursive
- name: Install packages
run: |
sudo apt-get update
Expand Down
5 changes: 3 additions & 2 deletions .gitmodules
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#
# SPDX-License-Identifier: MPL-2.0

[submodule "power-grid-model"]
path = power-grid-model
[submodule "deps/power-grid-model"]
path = deps/power-grid-model
url = [email protected]:PowerGridModel/power-grid-model.git
branch = main
6 changes: 5 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,11 @@ set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin)

include(GNUInstallDirs)

add_subdirectory("power-grid-model")
find_package(Eigen3 CONFIG REQUIRED)
find_package(Boost REQUIRED)
find_package(nlohmann_json CONFIG REQUIRED)
Comment on lines +32 to +34
Copy link
Member

Choose a reason for hiding this comment

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

not trying to revert this PR, but do you need to find these again? since the PGM core already hard-depends on this, you already have this from that location. Even more so, you're now actually overwriting the Eigen3_FOUND, Boost_FOUND and nlohmann_json_FOUND variables (and everything related). I think this is both redundant and actually more error-prone.


add_subdirectory("deps")

# add C library
add_subdirectory("power_grid_model_io_native_c")
Expand Down
6 changes: 6 additions & 0 deletions deps/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# SPDX-FileCopyrightText: Contributors to the Power Grid Model project <[email protected]>
#
# SPDX-License-Identifier: MPL-2.0

add_subdirectory("power-grid-model")

1 change: 1 addition & 0 deletions deps/power-grid-model
Submodule power-grid-model added at a05b88
1 change: 0 additions & 1 deletion power-grid-model
Submodule power-grid-model deleted from 560323
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@

add_library(power_grid_model_io_native INTERFACE)

target_link_libraries(power_grid_model_io_native INTERFACE power_grid_model)

target_include_directories(power_grid_model_io_native INTERFACE "${CMAKE_CURRENT_SOURCE_DIR}/include")

target_link_libraries(power_grid_model_io_native INTERFACE power_grid_model)
Laurynas-Jagutis marked this conversation as resolved.
Show resolved Hide resolved
Laurynas-Jagutis marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
// SPDX-FileCopyrightText: Contributors to the Power Grid Model project <[email protected]>
//
// SPDX-License-Identifier: MPL-2.0

#pragma once
#ifndef POWER_GRID_MODEL_IO_NATIVE_C_VNF_PGM_CONVERTER_HPP
#define POWER_GRID_MODEL_IO_NATIVE_C_VNF_PGM_CONVERTER_HPP

#include <power_grid_model/auxiliary/dataset.hpp>
#include <power_grid_model/common/exception.hpp>

#include <iostream>

class PgmVnfConverter {
public:
PgmVnfConverter(char* buffer = nullptr, power_grid_model::WritableDataset* data = nullptr);

// Public member functions
void parse_vnf_file();
power_grid_model::ConstDataset const* convert_input(power_grid_model::ConstDataset const* dataset);

private:
// Private attributes
char* f_file_buffer;
power_grid_model::WritableDataset*
deserialized_data; // this type because it is generated by a deserializer type structure

// Private setters/getters
void set_file_buffer(char* file_buffer);

void set_deserialized_data(power_grid_model::WritableDataset* deserialized_data);

char* get_file_buffer();

power_grid_model::WritableDataset* get_deserialized_data();

// Private member functions
void convert_node_input();
void convert_line_input();
void convert_sources_input();
void convert_sym_loads_input();
void convert_shunts_input();
void convert_transformer_input();
void convert_sym_gens_input();
void convert_links_input();
};

inline PgmVnfConverter::PgmVnfConverter(char* buffer, power_grid_model::WritableDataset* data)
Copy link
Member

Choose a reason for hiding this comment

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

is there a particular reason why you went with out-of-line implementations instead of in-class implementations (as we do in the PGM):

class PgmVnfConverter {
  public:
    PgmVnfConverter(char* buffer = nullptr, power_grid_model::WritableDataset* data = nullptr) 
        : f_file_buffer(buffer), deserialized_data(data) {
            using namespace std::string_literals;
            using power_grid_model::ExperimentalFeature;
            throw ExperimentalFeature{"PGM_VNF_converter", ExperimentalFeature::TypeValuePair{.name = "PGM_VNF_conversion",
                                                                                              .value = std::to_string(1)}};
    }

    // the rest of the functions
    // ...
};

: f_file_buffer(buffer), deserialized_data(data) {
using namespace std::string_literals;
using power_grid_model::ExperimentalFeature;
throw ExperimentalFeature{"PGM_VNF_converter", ExperimentalFeature::TypeValuePair{.name = "PGM_VNF_conversion",
.value = std::to_string(1)}};
}

inline void PgmVnfConverter::parse_vnf_file() {
// the function should use a deserializer type structure
// will be implemented later
}

inline power_grid_model::ConstDataset const*
PgmVnfConverter::convert_input(power_grid_model::ConstDataset const* /*dataset*/) {
convert_node_input();
convert_line_input();
convert_sources_input();
convert_sym_loads_input();
convert_shunts_input();
convert_transformer_input();
convert_sym_gens_input();
convert_links_input();

// then return the buffer
// return pgm_input_data;
// for now.
power_grid_model::ConstDataset* fake_data = nullptr;
return fake_data;
}

inline void PgmVnfConverter::set_file_buffer(char* file_buffer) { this->f_file_buffer = file_buffer; }

inline void PgmVnfConverter::set_deserialized_data(power_grid_model::WritableDataset* data) {
this->deserialized_data = data;
}

inline char* PgmVnfConverter::get_file_buffer() { return this->f_file_buffer; }

inline power_grid_model::WritableDataset* PgmVnfConverter::get_deserialized_data() { return this->deserialized_data; }

inline void PgmVnfConverter::convert_node_input() {
// Implementation
}

inline void PgmVnfConverter::convert_line_input() {
// Implementation
}

inline void PgmVnfConverter::convert_sources_input() {
// Implementation
}

inline void PgmVnfConverter::convert_sym_loads_input() {
// Implementation
}

inline void PgmVnfConverter::convert_shunts_input() {
// Implementation
}

inline void PgmVnfConverter::convert_transformer_input() {
// Implementation
}

inline void PgmVnfConverter::convert_sym_gens_input() {
// Implementation
}

inline void PgmVnfConverter::convert_links_input() {
// Implementation
}

inline void parse_vnf_file_wrapper(PgmVnfConverter* obj) { obj->parse_vnf_file(); }

inline power_grid_model::ConstDataset const* convert_input_wrapper(PgmVnfConverter* obj,
power_grid_model::ConstDataset const* dataset) {
return obj->convert_input(dataset);
}

#endif // POWER_GRID_MODEL_IO_NATIVE_C_VNF_PGM_CONVERTER_HPP
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
# C API library
add_library(power_grid_model_io_native_c SHARED
"src/handle.cpp"
"src/vnf_pgm_converter.cpp"
)

target_include_directories(power_grid_model_io_native_c PUBLIC
Expand All @@ -15,6 +16,7 @@ target_include_directories(power_grid_model_io_native_c PUBLIC
set(PGM_IO_NATIVE_PUBLIC_HEADERS
"${CMAKE_CURRENT_SOURCE_DIR}/include/power_grid_model_io_native_c/basics.h"
"${CMAKE_CURRENT_SOURCE_DIR}/include/power_grid_model_io_native_c/handle.h"
"${CMAKE_CURRENT_SOURCE_DIR}/include/power_grid_model_io_native_c/vnf_pgm_converter.h"
"${CMAKE_CURRENT_SOURCE_DIR}/include/power_grid_model_io_native_c.h"
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,6 @@

#include "power_grid_model_io_native_c/basics.h"
#include "power_grid_model_io_native_c/handle.h"
#include "power_grid_model_io_native_c/vnf_pgm_converter.h"

#endif // POWER_GRID_MODEL_IO_NATIVE_C_H
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,18 @@ typedef int32_t PGM_IO_ID;
// */
// typedef struct PGM_IO_VnfConverter PGM_IO_VnfConverter;

/**
* @brief Opaque struct for the VnfConverter class.
*
*/
typedef struct PGM_IO_VnfConverter PGM_IO_VnfConverter;

/**
* @brief Opaque struct for the ConstDataset.
*
*/
typedef struct PGM_IO_ConstDataset PGM_IO_ConstDataset;

/**
* @brief Opaque struct for the handle class.
*
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// SPDX-FileCopyrightText: Contributors to the Power Grid Model project <[email protected]>
//
// SPDX-License-Identifier: MPL-2.0

#pragma once
#ifndef POWER_GRID_MODEL_IO_NATIVE_C_VNF_PGM_CONVERTER_H
#define POWER_GRID_MODEL_IO_NATIVE_C_VNF_PGM_CONVERTER_H

#include "basics.h"

#ifdef __cplusplus
extern "C" {
#endif

/**
* @brief Create the PGM_VNF_converter
* @param handle
* @param file_buffer A pointer to the null-terminated C string.
* @return The pointer to a PGM_VNF_converter instance. The instance must be freed by
* PGM_VNF_delete_Converter
*/
PGM_IO_API PGM_IO_VnfConverter* PGM_VNF_create_converter(PGM_IO_Handle* handle, char* file_buffer);
Laurynas-Jagutis marked this conversation as resolved.
Show resolved Hide resolved

/**
* @brief Retrieve the transformed input data from .vnf format to PGM format
* @param handle
* @param converter_ptr A pointer to a PGM_VNF_converter instace.
* @param dataset A pointer to the const dataset supplied by the user.
* @return The pointer to the const dataset instance supplied by the user which has been filled in.
*/
PGM_IO_API PGM_IO_ConstDataset const* PGM_VNF_get_input_data(PGM_IO_Handle* handle, PGM_IO_VnfConverter* converter_ptr,
PGM_IO_ConstDataset const* dataset);

/**
* @brief Destroy the converter and free up the memory that was dedicated to it.
* @param converter_ptr A pointer to a PGM_VNF_converter instance.
*/
PGM_IO_API void PGM_VNF_delete_Converter(PGM_IO_VnfConverter* converter_ptr);
Copy link
Member

Choose a reason for hiding this comment

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

typo

Suggested change
PGM_IO_API void PGM_VNF_delete_Converter(PGM_IO_VnfConverter* converter_ptr);
PGM_IO_API void PGM_VNF_delete_converter(PGM_IO_VnfConverter* converter_ptr);


#ifdef __cplusplus
}
#endif

#endif // POWER_GRID_MODEL_IO_NATIVE_C_VNF_PGM_CONVERTER_H
Laurynas-Jagutis marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// SPDX-FileCopyrightText: Contributors to the Power Grid Model project <[email protected]>
//
// SPDX-License-Identifier: MPL-2.0

#define PGM_IO_DLL_EXPORTS

#include <power_grid_model_io_native/vnf_converter/vnf_pgm_converter.hpp>

#include "handle.hpp"
#include <power_grid_model_io_native_c/basics.h>
#include <power_grid_model_io_native_c/vnf_pgm_converter.h>

#include <power_grid_model/auxiliary/dataset.hpp>

using power_grid_model::ConstDataset;

// TODO(Laurynas-Jagutis) add call_with_catch for these functions
PGM_IO_VnfConverter* PGM_VNF_create_converter(const PGM_IO_Handle* /*handle*/, char* file_buffer) {
Laurynas-Jagutis marked this conversation as resolved.
Show resolved Hide resolved
auto* converter = new PgmVnfConverter(file_buffer);
parse_vnf_file_wrapper(converter);
return reinterpret_cast<PGM_IO_VnfConverter*>(converter);
Laurynas-Jagutis marked this conversation as resolved.
Show resolved Hide resolved
}

PGM_IO_ConstDataset const* PGM_VNF_get_input_data(const PGM_IO_Handle* /*handle*/, PGM_IO_VnfConverter* converter_ptr,
PGM_IO_ConstDataset const* dataset) {
auto* converter = reinterpret_cast<PgmVnfConverter*>(converter_ptr);
auto const* data = reinterpret_cast<ConstDataset const*>(dataset);
convert_input_wrapper(converter, data);
return reinterpret_cast<PGM_IO_ConstDataset const*>(data);
}

void PGM_VNF_delete_Converter(PGM_IO_VnfConverter* converter_ptr) {
auto* converter = reinterpret_cast<PgmVnfConverter*>(converter_ptr);
delete converter;
}
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ ignore_missing_imports = true
show_column_numbers = true
non_interactive = true
install_types = true
exclude = "power-grid-model/"
exclude = "deps/power-grid-model/"

# CI build options
[tool.cibuildwheel]
Expand Down
3 changes: 2 additions & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,8 @@ def generate_build_ext(pkg_dir: Path, pkg_name: str):
pgm_io_c = Path("power_grid_model_io_native_c")
pgm = Path("power_grid_model")
pgm_c = Path("power_grid_model_c")
pgm_pkg_dir = pkg_dir / str(pgm).replace("_", "-")
deps = Path("deps")
pgm_pkg_dir = pkg_dir / deps / str(pgm).replace("_", "-")

# include-folders
include_dirs = [
Expand Down
1 change: 1 addition & 0 deletions sonar-project.properties
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ sonar.sourceEncoding=UTF-8
sonar.issue.ignore.allfile=a1
sonar.issue.ignore.allfile.a1.fileRegexp='.*#include.*doctest\.h[>"].*'
sonar.coverage.exclusions="tests/**/*"
sonar.coverage.exclusions=power_grid_model_io_native_c/power_grid_model_io_native/include/power_grid_model_io_native/vnf_converter/vnf_pgm_converter.hpp,power_grid_model_io_native_c/power_grid_model_io_native_c/src/vnf_pgm_converter.cpp
Copy link
Contributor

Choose a reason for hiding this comment

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

I am unsure if this is the way to go, because maybe later it can be forgotten and once implementation is ready some tests missed altogether. Just food for thought.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a temporary exclusion as the details are not implemented in the skeleton. The exclusions will be removed later on as the code are covered.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a todo here as well:

Suggested change
sonar.coverage.exclusions=power_grid_model_io_native_c/power_grid_model_io_native/include/power_grid_model_io_native/vnf_converter/vnf_pgm_converter.hpp,power_grid_model_io_native_c/power_grid_model_io_native_c/src/vnf_pgm_converter.cpp
# TODO(Laurynas-Jagutis) remove temporary scan disables
sonar.coverage.exclusions=power_grid_model_io_native_c/power_grid_model_io_native/include/power_grid_model_io_native/vnf_converter/vnf_pgm_converter.hpp,power_grid_model_io_native_c/power_grid_model_io_native_c/src/vnf_pgm_converter.cpp

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I forgot to unresolve this thread when commenting. Please don't forget to add one as well in the future PR's.

sonar.cpd.exclusions="tests/**/*"
sonar.cfamily.threads=1
sonar.coverageReportPaths=cpp_coverage.xml
Expand Down
1 change: 1 addition & 0 deletions tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ find_package(doctest REQUIRED)
include("${doctest_DIR}/doctest.cmake")

add_subdirectory("c_api_tests")
add_subdirectory("cpp_unit_tests")
Loading
Loading