-
Notifications
You must be signed in to change notification settings - Fork 46
Feature: support multi-dimensional batch calculation[experimental] #1186
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
base: main
Are you sure you want to change the base?
Changes from all commits
8085e5b
73d85c5
234f38d
be44d80
1fd7ed3
45f4d72
06cb330
a4fc01e
118655f
3b41f28
d88db97
1b1190d
21c31c5
6e8c081
0b989b4
14b4039
3338881
01c86a9
7786b0c
5a3f394
2650968
9aaa6bd
e4aa439
237681f
b602f2e
3e79237
8d8d80c
61cfd57
a1331ba
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -287,6 +287,9 @@ template <dataset_type_tag dataset_type_> class Dataset { | |
| } | ||
| constexpr bool is_dense(Idx const i) const { return is_dense(buffers_[i]); } | ||
| constexpr bool is_dense(Buffer const& buffer) const { return buffer.indptr.empty(); } | ||
| constexpr bool is_dense() const { | ||
| return std::ranges::all_of(buffers_, [this](Buffer const& buffer) { return is_dense(buffer); }); | ||
| } | ||
| constexpr bool is_sparse(std::string_view component, bool with_attribute_buffers = false) const { | ||
| Idx const idx = find_component(component, false); | ||
| if (idx == invalid_index) { | ||
|
|
@@ -485,7 +488,7 @@ template <dataset_type_tag dataset_type_> class Dataset { | |
| Dataset get_individual_scenario(Idx scenario) const | ||
| requires(!is_indptr_mutable_v<dataset_type>) | ||
| { | ||
| assert(0 <= scenario && scenario < batch_size()); | ||
| assert(0 <= scenario && scenario <= batch_size()); | ||
|
|
||
| Dataset result{*this}; | ||
| result.dataset_info_.is_batch = false; | ||
|
|
@@ -510,6 +513,27 @@ template <dataset_type_tag dataset_type_> class Dataset { | |
| return result; | ||
| } | ||
|
|
||
| // get slice dataset from batch | ||
| Dataset get_slice_scenario(Idx begin, Idx end) const | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. really useful 👍 If we add an additional |
||
| requires(!is_indptr_mutable_v<dataset_type>) | ||
| { | ||
| assert(begin <= end); | ||
| assert(0 <= begin); | ||
| assert(end <= batch_size()); | ||
| assert(is_batch()); | ||
| assert(is_dense()); | ||
|
|
||
| Dataset result = get_individual_scenario(begin); | ||
| Idx const batch_size = end - begin; | ||
| result.dataset_info_.is_batch = true; | ||
| result.dataset_info_.batch_size = batch_size; | ||
| for (auto&& [buffer, component_info] : std::views::zip(result.buffers_, result.dataset_info_.component_info)) { | ||
| Idx const size = component_info.elements_per_scenario * batch_size; | ||
| component_info.total_elements = size; | ||
| } | ||
| return result; | ||
| } | ||
|
|
||
| private: | ||
| MetaData const* meta_data_; | ||
| DatasetInfo dataset_info_; | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -137,6 +137,12 @@ typedef struct PGM_WritableDataset PGM_WritableDataset; | |||||
| * @brief Opaque struct for the information of the dataset. | ||||||
| */ | ||||||
| typedef struct PGM_DatasetInfo PGM_DatasetInfo; | ||||||
|
|
||||||
| /** | ||||||
| * @brief Opaque struct for the multi dimensional dataset class. | ||||||
| * | ||||||
| */ | ||||||
| typedef struct PGM_MultiDimensionalDataset PGM_MultiDimensionalDataset; | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe following the conventions of
Suggested change
|
||||||
| #endif | ||||||
|
|
||||||
| // NOLINTEND(modernize-use-using) | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -309,6 +309,36 @@ PGM_API void PGM_dataset_mutable_add_attribute_buffer(PGM_Handle* handle, PGM_Mu | |
| */ | ||
| PGM_API PGM_DatasetInfo const* PGM_dataset_mutable_get_info(PGM_Handle* handle, PGM_MutableDataset const* dataset); | ||
|
|
||
| /** | ||
| * @brief Create a PGM_MultiDimensionalDataset from multiple PGM_ConstDataset instances | ||
| * | ||
| * @param handle | ||
| * @param const_datasets | ||
| * @param n_datasets | ||
| * @return | ||
| */ | ||
| PGM_API PGM_MultiDimensionalDataset* | ||
| PGM_dataset_create_multidimensional_from_const(PGM_Handle* handle, PGM_ConstDataset const** const_datasets, | ||
| PGM_Idx n_datasets); | ||
|
|
||
| /** | ||
| * @brief Get the array pointer from a PGM_MultiDimensionalDataset | ||
| * | ||
| * @param handle | ||
| * @param multidimensional_dataset | ||
| * @return | ||
| */ | ||
| PGM_API PGM_ConstDataset const* | ||
| PGM_get_array_pointer_from_multidimensional(PGM_Handle* handle, | ||
| PGM_MultiDimensionalDataset const* multidimensional_dataset); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe we can do a multi-indexing approach instead? e.g. PGM_mddataset_get_data(PGM_Handle* handle, PGM_MDDataset const* mddataset);
PGM_mddataset_get_flat_element(PGM_Handle* handle, PGM_MDDataset const* mddataset, PGM_Idx flattened_index);
PGM_mddataset_get_scenario(PGM_Handle* handle, PGM_MDDataset const* mddataset, PGM_Idx** multi_index, PGM_Idx n_dimensions);(we can even omit |
||
|
|
||
| /** | ||
| * @brief destroy the multidimensional dataset object | ||
| * | ||
| * @param multidimensional_dataset | ||
| */ | ||
| PGM_API void PGM_destroy_multidimensional_dataset(PGM_MultiDimensionalDataset* multidimensional_dataset); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. inconsistent |
||
|
|
||
|
Comment on lines
+312
to
+341
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mgovers This is one option to pass a MD datasets into the C-API. You need to have additional helper functions to create an array of datasets. We need a new opaque struct. Another interesting approach would be using chaining. So define a function called
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i prefer this separate ND approach, but how would we use ND-datasets, e.g. a 3-dimensional one instead of only 2-dimensional
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is not really a ND, but a list of datasets. The core will make a cross-join ND calculation of combinations in the list. The questions, do we create separate opaque struct to store an array of datasets? This requires a lot of new functions. Or do we do the chaining, this only needs one new function.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i do believe under the current implementation, sure, but it also restricts us going forward. i feel like if we are going to support 1D and have a separate object for 2D, then we might as well go for ND. otherwise, we'll fall into the same rabbit hole that C++ has been in (watch any video on
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The current experiment is about ND, not just 2D. The ND calculation is created by a list of datasets. Each dataset in the list represents the mutation of that dimension. The core creates a cross-join on the mutations. |
||
| #ifdef __cplusplus | ||
| } | ||
| #endif | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,8 @@ | |
|
|
||
| #include <power_grid_model/auxiliary/dataset_fwd.hpp> | ||
|
|
||
| #include <vector> | ||
|
|
||
| // forward declare all referenced struct/class in C++ core | ||
| // alias them in the root namespace | ||
|
|
||
|
|
@@ -36,3 +38,4 @@ using PGM_ConstDataset = power_grid_model::meta_data::Dataset<power_grid_model:: | |
| using PGM_MutableDataset = power_grid_model::meta_data::Dataset<power_grid_model::mutable_dataset_t>; | ||
| using PGM_WritableDataset = power_grid_model::meta_data::Dataset<power_grid_model::writable_dataset_t>; | ||
| using PGM_DatasetInfo = power_grid_model::meta_data::DatasetInfo; | ||
| using PGM_MultiDimensionalDataset = std::vector<PGM_ConstDataset>; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i feel like this should be able to use arbitrary dimensions
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. C++23 has |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think this will crash because
buffer.indptr[scenario + 1]is called. In python, it's also not possible to call[1, 2, 3][3](IndexOutOfRange)