Implementation of feature 720 asym_line #762
Conversation
|
Hi @leovsch, thanks for the draft PR. I have put the check list in the description of the PR to track where we are. |
|
Hi @leovsch, Herewith I come back with some suggestions on the implementation of Class memberI suggest having the following two class members for the parameters. The double i_n_;
double base_i_;
ComplexTensor<asymmetric_t> y_series_;
ComplexTensor<asymmetric_t> y_shunt_;ConstructionThe construction of For the construction of if (is_nan(r_na)) {
// if r_na is nan, it means user already does the kron reduction
// we just assign the 3*3 z_series by fill-in with (r + x * imaginary_unit) for each entry
// also fill-in the upper triangle based on symmetry.
}
else {
// if r_na is not nan, the user specifies the neutral conductor parameters, we need to do kron reduction
// in a way that
// z_kl_reduced = z_kl - z_nk * z_nl / z_nn
// k, l = a, b, c
// note: z_nk = z_kn, etc.
// fill-in all the values of 3*3 z_series, also the symmetry
}
y_series_ = inverse(z_series);We do the similar for Calculation parametersSymmetric parameterTo implement y_s = average of diagonal of y_series_;
y_m = average of off-diagonal of y_series_;
y1_series = y_s - y_mYou put the Asymmetric parameterTo implement Please learn the function |
TonyXiang8787
changed the title
|
First implementation version is here some general remarks and questions:
I like working on the project, hope my questions are clear otherwise reach out 👍 |
|
Hi @leovsch , I will most likely spend some time tomorrow to review this, but at first glance, you've made the right decisions and followed the correct path. Without digging more into it, here's some initial response on your questions.
we try to follow the paradigm "If it looks like a duck, swims like a duck, and quacks like a duck, then it probably is a duck", especially on the things in
we do have something similar for transformers. That functionality resides in
I put it as a topic on the agenda for tomorrow with the rest of the team.
If you like, we can have a call about this. We do have some guidelines, but also a lot of heuristics, so it might be easier to talk to you directly. You can send an email to me (martijn.govers@alliander.com) if that is alright with you, and we can schedule a meeting.
That's great to hear! It's very nice to see external contributors working on the project, and we're happy to help you out at any time. Any feedback you have is also highly valued. Again, thank you for your input so far! Martijn |
Thanks again for the contribution! |
| Eigen::Array<DoubleComplex, 1, 3> Y_ba; | ||
| Y_ba << Y(3,0), Y(3,1), Y(3,2); | ||
| DoubleComplex Y_bb_inv = 1.0 / Y(3,3); | ||
|
|
||
| return Y_aa - ((Y_ab * Y_bb_inv).matrix() * Y_ba.matrix()).array(); |
There was a problem hiding this comment.
Its always tricky with eigen! Is this what you intended to do?
| Eigen::Array<DoubleComplex, 1, 3> Y_ba; | |
| Y_ba << Y(3,0), Y(3,1), Y(3,2); | |
| DoubleComplex Y_bb_inv = 1.0 / Y(3,3); | |
| return Y_aa - ((Y_ab * Y_bb_inv).matrix() * Y_ba.matrix()).array(); | |
| ComplexValue<asymmetric_t> Y_ba(Y(3,0), Y(3,1), Y(3,2)); | |
| DoubleComplex Y_bb_inv = 1.0 / Y(3,3); | |
| return Y_aa - vector_outer_product(Y_ba, Y_ab) * Y_bb_inv; |
There was a problem hiding this comment.
Ah yes I needed a row times column vector implementation, I'll change it to the vector_outer_product function
| DoubleComplex a = std::pow(e, 1.0i * (2.0 / 3.0) * pi); | ||
| ComplexTensor<asymmetric_t> a_matrix = ComplexTensor<asymmetric_t>(1, 1, pow(a, 2), 1, a, pow(a, 2)); | ||
| ComplexTensor<asymmetric_t> a_matrix_inv = (1.0/3.0) * ComplexTensor<asymmetric_t>(1, 1, a, 1, pow(a, 2), a); | ||
| ComplexTensor<asymmetric_t> z_series = (a_matrix_inv.matrix() * z_series_abc.matrix() * a_matrix.matrix()).array(); | ||
| return z_series; |
There was a problem hiding this comment.
We have get_sym_matrix() and get_sym_matrix_inv() available for the transformation matrix.
| @@ -56,6 +56,7 @@ using DoubleComplex = std::complex<double>; | |||
| using std::numbers::inv_sqrt3; | |||
| using std::numbers::pi; | |||
| using std::numbers::sqrt3; | |||
| using std::numbers::e; | |||
There was a problem hiding this comment.
with this being in the full namespace of this project, i'm a little bit hesitant to add single-letter names here. let's think about whether this is the way to go or whether it's preferable to just use std::numbers::e where it's used (since it's not that common in the code base anyways). alternatively, we can put them all in a power_grid_model::numbers namespace and expose only the longer ones outside.
namespace numbers {
using std::numbers::inv_sqrt3;
using std::numbers::pi;
using std::numbers::sqrt3;
using std::numbers::e;
} // namespace numbers
using numbers::inv_sqrt3;
using numbers::pi;
using numbers::sqrt3;| class UnsupportedInputDescriptionAsymLine : public PowerGridError { | ||
| public: | ||
| UnsupportedInputDescriptionAsymLine() { | ||
| append_msg("Invalid or missing parameters supplied for component asym_line. The following input specifications are allowed"); | ||
| append_msg("3 phase x_matrix, 3 phase r_matrix and 3 phase c_matrix"); | ||
| append_msg("3 phase + neutral x_matrix, 3 phase + neutral r_matrix and 3 phase + neutral c_matrix"); | ||
| append_msg("3 phase x_matrix, 3 phase r_matrix and c1, c0"); | ||
| append_msg("3 phase + neutral x_matrix, 3 phase + neutral r_matrix and c1, c0"); | ||
| } | ||
| }; |
There was a problem hiding this comment.
i don't think we explicitly check input parameters in any of the other components. only things like ID references to other components are checked. I would just consider this UB (undefined behavior; in this case in particular unspecified behavior). The data validator in Python can check that it is supported, but the core should not.
| @@ -61,6 +62,8 @@ template <scalar_value T> class Tensor : public Eigen3Tensor<T> { | |||
| // additional constructors | |||
| explicit Tensor(T const& x) { (*this) << x, 0.0, 0.0, 0.0, x, 0.0, 0.0, 0.0, x; } | |||
| explicit Tensor(T const& s, T const& m) { (*this) << s, m, m, m, s, m, m, m, s; } | |||
| explicit Tensor(T const& x1, T const& x2, T const& x3, T const& x4, T const& x5, T const& x6) { (*this) << x1, x2, x4, x2, x3, x5, x4, x5, x6; } | |||
There was a problem hiding this comment.
i'd go for something more explicit. It's very easy to make a mistake in the order.
A proposal is the one below (BEWARE: this changes the order, so usage needs to be adjusted)
| explicit Tensor(T const& x1, T const& x2, T const& x3, T const& x4, T const& x5, T const& x6) { (*this) << x1, x2, x4, x2, x3, x5, x4, x5, x6; } | |
| explicit Tensor(T const& s1, T const& s2, T const& s3, T const& m12, T const& m13, T const& m23) { (*this) << s1, m12, m13, m12, s2, m23, m13, m23, s3; } |
alternatively, you can provide 2 Vector<T> as input for readability
| // additional constructors | ||
| explicit Tensor4(T const& x) { (*this) << x, 0.0, 0.0, 0.0, 0.0, x, 0.0, 0.0, 0.0, 0.0, x, 0.0, 0.0, 0.0, 0.0, x; } | ||
| explicit Tensor4(T const& s, T const& m) { (*this) << s, m, m, m, m, s, m, m, m, m, s, m, m, m, m, s; } | ||
| explicit Tensor4(T const& x1, T const& x2, T const& x3, T const& x4, T const& x5, T const& x6, T const& x7, T const& x8, T const& x9, T const& x10) { (*this) << x1, x2, x4, x7, x2, x3, x5, x8, x4, x5, x6, x9, x7, x8, x9, x10; } |
| ComplexTensor<asymmetric_t> Y_aa = ComplexTensor<asymmetric_t>(Y(0,0), Y(1,0), Y(1, 1), Y(2,0), Y(2,1), Y(2,2)); | ||
| ComplexValue<asymmetric_t> Y_ab(Y(0,3), Y(1,3), Y(2,3)); | ||
| Eigen::Array<DoubleComplex, 1, 3> Y_ba; | ||
| Y_ba << Y(3,0), Y(3,1), Y(3,2); |
There was a problem hiding this comment.
you can initialize it as a ComplexValue<asymmetric_t>, right? or even do (untested/might be syntactically incorrect) ComplexValue<asymmetric_t> Y_ba{Y.row(3).transpose()};
| } | ||
| else { |
There was a problem hiding this comment.
formatting can be automatically solved using pre-commit (see https://power-grid-model.readthedocs.io/en/stable/contribution/CONTRIBUTING.html#pre-commit-hooks ; you can apply the format changes retroactively using pre-commit run --all-files and run it before committing using pre-commit install)
| } | |
| else { | |
| } else { |
| return param; | ||
| } | ||
| }; | ||
| } |
There was a problem hiding this comment.
code convention: newline at end of file (EOF)
| } | |
| } | |
| } | ||
|
|
||
| DoubleComplex average_of_diagonal_of_matrix(const ComplexTensor<asymmetric_t> &matrix) const { | ||
| return (matrix(0,0) + matrix(1,1) + matrix(2,2)) / 3.0; |
There was a problem hiding this comment.
i think we already have functionality in three_phase_tensor.hpp for this. not sure, though
| } | ||
| } | ||
|
|
||
| } // namespace power_grid_model |
There was a problem hiding this comment.
newline at EOF
| } // namespace power_grid_model | |
| } // namespace power_grid_model | |
| ComplexTensor4 y = r_matrix + 1.0i * x_matrix; | ||
| z_series_abc = kron_reduction(y); | ||
| } | ||
| DoubleComplex a = std::pow(e, 1.0i * (2.0 / 3.0) * pi); |
There was a problem hiding this comment.
instead of using std::pow(e, ...), please use std::exp (https://en.cppreference.com/w/cpp/numeric/math/exp). as a result, you may even remove the using declaration from the common.hpp
@leovsch I have compared with OpenDSS and I could not find the different you mentioned. Maybe can you elaborate a bit more about what exactly are we still missing? Then we can make a choice. |
| } | ||
|
|
||
| DoubleComplex average_of_off_diagonal_of_matrix(const ComplexTensor<asymmetric_t> &matrix) const { | ||
| return (matrix(0,2) + matrix(1,1) + matrix(2,0)) / 3.0; |
There was a problem hiding this comment.
| return (matrix(0,2) + matrix(1,1) + matrix(2,0)) / 3.0; | |
| return (matrix(0,1) + matrix(0,2) + matrix(1,0) + matrix(1,2) + matrix(2,0) + matrix(2,1)) / 6.0; |
Hi Tony, In OpenDSS you can per parameter (R, X, C) decide if you want to supply the full matrix or just the 0th and 1st order values. Example 1: Example 2: And all posible other permutations of the input format. For now I've implemented only the possibility that would support example 1. I can imagine that you would want to generalize such functionality to all possible permutations of the input format just as in OpenDSS. Hope this clarifies it a bit for you. |
@leovsch This is a valid point. Thanks for clarification. We need to discuss this and come back to you later. |
Hi @leovsch, Comeback to your point, we have thoroughly considered different options and decide to go with the attributes you have already defined in this PR. Concretely means: R = always full r matrix (no r1, r0 possible) From practical applications, if the user want to specify a r1/r0, they will almost certain specify x1/x0 instead of full x matrix, verse versa is also true. In that case, the user can just use normal We allow C to be specified in both c0/c1 or full c matrix way because this could be a use-case for the user. |
Thanks for the clarifification. I will leave it as is. |
Implement feature:
# 720Changes proposed in this PR include:
Implementation of feature 720.
Add a new component to support asym_line specification with R, X and C matrix values
Could you please pay extra attention to the points below when reviewing the PR:
I'm a new contributor so pay attention to the coding standards applicable to this project along with achitectural rules etc.
Check list
power-grid-model/power_grid_model_c/power_grid_model/include/power_grid_model/auxiliary/input.hpp/update.hpp/output.hppor in the documentation directly)code_generation/data/attribute_classes/input.json/update.json/output.json+ runcode_generation/code_gen.pypower-grid-model/power_grid_model_c/power_grid_model/include/power_grid_model/component/[component].hppfile that at least inherits from Base, but in this caseGenericBranchshould inherit fromBranchpower-grid-model/tests/cpp_unit_tests/test_[component].cpptest_[component].cpptopower-grid-model/tests/cpp_unit_tests/CMakeLists.txtpower_grid_model_c/power_grid_model/include/power_grid_model/all_components.hppmain_core/topology.hpp/input.hpp/output.hpp/update.hpp)code_generation/data/dataset_class_maps/dataset_definitions.json+ re-runcode_generation/code_gen.pytests/datasrc/power_grid_model/validation/validation.py+ add corresponding tests