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.

Sign up for GitHub

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

Jump to bottom

Remove the Dataset logic from PGM core, use DatasetHandler for MainModel #431

Closed
TonyXiang8787 opened this issue Nov 21, 2023 · 2 comments · Fixed by #587
Closed

Remove the Dataset logic from PGM core, use DatasetHandler for MainModel #431

TonyXiang8787 opened this issue Nov 21, 2023 · 2 comments · Fixed by #587
Labels
improvement Improvement on internal implementation

Comments

@TonyXiang8787
Copy link
Member

TonyXiang8787 commented Nov 21, 2023 •
edited

Background

Currently we use Dataset as the input view for MainModel, such as:

power-grid-model/power_grid_model_c/power_grid_model/include/power_grid_model/main_model.hpp

Lines 101 to 102 in 9521280

explicit MainModelImpl(double system_frequency, ConstDataset const& input_data, Idx pos = 0)
: system_frequency_{system_frequency} {

The C-API receives DatasetHandler, we have a function to export to a Dataset. See blow:

power-grid-model/power_grid_model_c/power_grid_model/include/power_grid_model/auxiliary/dataset_handler.hpp

Lines 69 to 71 in 9521280

template <bool dataset_const>
std::map<std::string, DataPointer<dataset_const>> export_dataset(Idx scenario = -1) const
requires(dataset_const || data_mutable)

This conversion is redundant. We already have DatasetHandler which handles datasets.

Adjustments

  1. Create export function in DatasetHandler to export a std::span for a single component.
  2. Remove export_dataset function in DatasetHandler.
  3. Change the constructor, get output, and update function of MainModel to receive range of a certain component type.
  4. Create a new class OwningDataset to contain owned buffers. This can be used for the validation test and later for the serialization-to-serialization API for the C-API. (msgpack input -> msgpack output).
@TonyXiang8787 TonyXiang8787 added feature New feature or request improvement Improvement on internal implementation and removed feature New feature or request labels Nov 21, 2023
@mgovers
Copy link
Member

mgovers commented Feb 14, 2024

@TonyXiang8787 how much of this is still relevant / unblocked with the new constexpr metadata from #475 ?

@TonyXiang8787
Copy link
Member Author

@mgovers still quite relevant

@TonyXiang8787 TonyXiang8787 mentioned this issue Mar 26, 2024
Open
@TonyXiang8787 TonyXiang8787 mentioned this issue May 6, 2024
Merged
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
improvement Improvement on internal implementation
Projects
Power Grid Model
Status: Q2 2024
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Feature/remove legacy dataset PowerGridModel/power-grid-model
2 participants
@mgovers @TonyXiang8787