Feature / Add columnar data support in main model

[Jerry-Jinfeng-Guo]

• input
• output
• update
• integration test
  data id field inference

[TonyXiang8787]

@Jerry-Jinfeng-Guo @mgovers @figueroa1395, from the user's perspective, the user would definitely like to provide a columnar batch dataset in a way that the id is not provided for a certain component. In that case, it should be inferred that the elements where attributes are to be updated via columnar buffer are in the exact same sequence of the input data. This is a realistic use-case and will be appreciated by the user, to save the additional step to just assign the exactly the same id as in the input data. The following Python code should work:

model = PowerGridModel(input_data=input_data)
result = model.calculate_power_flow(
    update_data={
        "sym_load": {"p_specified": np.random.randn(n_step, n_sym_load)}
    }
)

In the main core, we need to have special treatment in is_update_independent to make this work:

1. is_update_independent should be per component instead of the whole dataset. So we can allow individual sequence for each component.
2. For a certain component, if the buffer is row-based, we use the same logic as is now.
3. For a certain component, if the buffer is columnar, we do the following:
   1. If id attribute buffer is provided, we look at id to judge if the component is independent or not. We do not need to create proxy stuff which is waste of time. Just directly look at id buffer.
   2. If id attribute buffer is not provided:
      1. If the buffer is not uniform, or the buffer is uniform but elements_per_scenario is not the same as the number of elements in the input data (in the model). An error should be raised.
      2. If the above check passes, we assume the component buffer is independent. And we generate a sequence from 0 to n_comp for this component. This will be consumed by the update function so the update function does not do id lookup.

[mgovers]

@Jerry-Jinfeng-Guo @mgovers @figueroa1395, from the user's perspective, the user would definitely like to provide a columnar batch dataset in a way that the id is not provided for a certain component. In that case, it should be inferred that the elements where attributes are to be updated via columnar buffer are in the exact same sequence of the input data. This is a realistic use-case and will be appreciated by the user, to save the additional step to just assign the exactly the same id as in the input data. The following Python code should work:

model = PowerGridModel(input_data=input_data)
result = model.calculate_power_flow(
    update_data={
        "sym_load": {"p_specified": np.random.randn(n_step, n_sym_load)}
    }
)

In the main core, we need to have special treatment in is_update_independent to make this work:

1. is_update_independent should be per component instead of the whole dataset. So we can allow individual sequence for each component.

2. For a certain component, if the buffer is row-based, we use the same logic as is now.

3. For a certain component, if the buffer is columnar, we do the following:

   1. If id attribute buffer is provided, we look at id to judge if the component is independent or not. We do not need to create proxy stuff which is waste of time. Just directly look at id buffer.

   2. If id attribute buffer is not provided:

      1. If the buffer is not uniform, or the buffer is uniform but elements_per_scenario is not the same as the number of elements in the input data (in the model). An error should be raised.
      2. If the above check passes, we assume the component buffer is independent. And we generate a sequence from 0 to n_comp for this component. This will be consumed by the update function so the update function does not do id lookup.

I do think it's very valuable, but I also believe that it's a separate feature request and definitely out of scope of this PR

[TonyXiang8787]

@Jerry-Jinfeng-Guo @mgovers @figueroa1395, from the user's perspective, the user would definitely like to provide a columnar batch dataset in a way that the id is not provided for a certain component. In that case,
...

I do think it's very valuable, but I also believe that it's a separate feature request and definitely out of scope of this PR

I agree to put this as a separate PR. But this feature needs to be part of the release 1.10.0, including its documentation and examples. I have now edited in #548 as step 7 of the task.

[figueroa1395]

Looks good to me, but I have a final question to get the full picture better:

Was the introduction of all iterator_like stuff necessary because the columnar stuff was implemented using boost iterator stuff? And since this boost iterator stuff doesn't fully support all the std::iterator concepts, then you had to mock it? And that is the reason why extending the use of columnar in main model was not trivial?

[Jerry-Jinfeng-Guo]

I think the subsequent changes make sense to me. I can not approve this PR since I created it. Could someone else give the approval if it looks good to you as well? @TonyXiang8787 @figueroa1395 @nitbharambe

[mgovers]

Looks good to me, but I have a final question to get the full picture better:

Was the introduction of all iterator_like stuff necessary because the columnar stuff was implemented using boost iterator stuff? And since this boost iterator stuff doesn't fully support all the std::iterator concepts, then you had to mock it? And that is the reason why extending the use of columnar in main model was not trivial?

yes indeed. The fundamental reason is that std::iterator requires you to return an exact reference, but a view only returns something that acts like a reference but is not necessarily one.

[Jerry-Jinfeng-Guo]

Was the introduction of all iterator_like stuff necessary because the columnar stuff was implemented using boost iterator stuff?

Since @mgovers made that change, I could only answer from my understanding. Making it typename ForwardIterator is technically working but provides much less control for us than ideal (the real forward iterator). Having one temporary one in place made it easier to switch once in the future said functionality is fully supported by boost.

[figueroa1395]

Since @mgovers made that change, I could only answer from my understanding. Making it typename ForwardIterator is technically working but provides much less control for us than ideal (the real forward iterator). Having one temporary one in place made it easier to switch once in the future said functionality is fully supported by boost.

But would it work though? I think not, because the boost iterator stuff doesn't yet provide range access as required in the newly introduced concepts. Specifically this:

yes indeed. The fundamental reason is that std::iterator requires you to return an exact reference, but a view only returns something that acts like a reference but is not necessarily one.

Hence triggering the changes from std::convertible_to to detail::assignable_to

But indeed this change would be easy to change once boost does.

I think the subsequent changes make sense to me. I can not approve this PR since I created it. Could someone else give the approval if it looks good to you as well? @TonyXiang8787 @figueroa1395 @nitbharambe

I will give approval. Feel free to resolve remaining open conversations.

[mgovers]

But would it work though? I think not, because the boost iterator stuff doesn't yet provide range access as required in the newly introduced concepts.

std::views::iota coming to the rescue for the IdxCount issue

[sonarqubecloud]

Quality Gate passed

Issues
3 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud