Measurement statistics module #896

nitbharambe · 2025-02-13T15:10:58Z

Implements variance related calculations mentioned in # #547

Changes proposed in this PR include:

A generic module for handling different types of variances in state estimation.

Could you please pay extra attention to the points below when reviewing the PR:

Signed-off-by: Martijn Govers <[email protected]>

Signed-off-by: Nitish Bharambe <[email protected]>

mgovers · 2025-02-17T10:43:10Z

power_grid_model_c/power_grid_model/include/power_grid_model/common/statistics.hpp

+    explicit operator UniformComplexRDV<sym>() const {
+        return UniformComplexRDV<sym>{.value = value, .variance = sum_val(variance)};
+    }


here and elsewhere: please double-check whether additional conversion operators are required

I added tests without checking implementations. Also it would be nice to now add additional conversion operators if there is a need for it during implementation. The base for the operators are ready and available in this PR.

so do you want me to review as is? i would be reviewing my own code though 😬

Signed-off-by: Nitish Bharambe <[email protected]>

mgovers

from a very quick look it seems OK to me, but maybe it's good to let someone else review, as i've written a significant part of the code

mgovers · 2025-02-18T14:55:43Z

power_grid_model_c/power_grid_model/include/power_grid_model/common/statistics.hpp

+    explicit operator UniformComplexRDV<sym>() const {
+        return UniformComplexRDV<sym>{.value = value, .variance = sum_val(variance)};
+    }


so do you want me to review as is? i would be reviewing my own code though 😬

Signed-off-by: Nitish Bharambe <[email protected]>

nitbharambe · 2025-02-18T15:35:54Z

from a very quick look it seems OK to me, but maybe it's good to let someone else review, as i've written a significant part of the code

True, asked @Jerry-Jinfeng-Guo .

Signed-off-by: Nitish Bharambe <[email protected]>

sonarqubecloud · 2025-02-18T20:49:26Z

Quality Gate passed

Issues
2 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 SonarQube Cloud

Jerry-Jinfeng-Guo

Couple comments on the file statistics.hpp in general before proceeding to the test file:

Please rename RDV to RandVar
Please rename Uniform to Unified
Is the symmetric member variable actually useful?
Shall we add some simple equations as reference to the conversions between the forms?

Jerry-Jinfeng-Guo · 2025-02-19T10:12:15Z

power_grid_model_c/power_grid_model/include/power_grid_model/common/statistics.hpp

+/**
+ * @file statistics.hpp
+ * @brief This file contains various structures and functions for handling statistical representations of
+ * randomly distributed variables(RDV) used in the Power Grid Model, like in the State Estimation algorithms to


randomly distributed variable is quite a strange expression. For this specific context, 'random variable' is accurate and comcise enough.

Suggested change

* randomly distributed variables(RDV) used in the Power Grid Model, like in the State Estimation algorithms to

* random variables (RandVar) used in the Power Grid Model, like in the State Estimation algorithms to

or

Suggested change

* randomly distributed variables(RDV) used in the Power Grid Model, like in the State Estimation algorithms to

* random variables (RV) used in the Power Grid Model, like in the State Estimation algorithms to

Jerry-Jinfeng-Guo · 2025-02-19T10:13:13Z

power_grid_model_c/power_grid_model/include/power_grid_model/common/statistics.hpp

+ * with different types of variances. These structures support both symmetric and asymmetric representations and
+ * provide conversion operators to transform between these representations.
+ *
+ * A Randomly distributed variable in PGM can have following characteristics:


Suggested change

* A Randomly distributed variable in PGM can have following characteristics:

* A random variable in PGM can have following characteristics:

Jerry-Jinfeng-Guo · 2025-02-19T10:25:53Z

power_grid_model_c/power_grid_model/include/power_grid_model/common/statistics.hpp

+ *  - Uniform: Single Variance for all phases
+ *  - Independent: Unique Variance for each phase
+ *  - Real: The Real value without direction, eg. real axis: RealValue (* 1), imaginary axis: RealValue (* 1i).
+ *  - Complex: A combined complex value in `a + bi` notation.


Suggested change

* - Uniform: Single Variance for all phases

* - Independent: Unique Variance for each phase

* - Real: The Real value without direction, eg. real axis: RealValue (* 1), imaginary axis: RealValue (* 1i).

* - Complex: A combined complex value in `a + bi` notation.

* - Unified: total variance for all phases

* - Independent: all phases are independent from each other

* - Scalar: a scalar value `RealValue`, eg. real axis: RealValue (* 1), imaginary axis: RealValue (* i).

* - Complex: a complex value with real and imaginary parts.

Jerry-Jinfeng-Guo · 2025-02-19T10:30:52Z

power_grid_model_c/power_grid_model/include/power_grid_model/common/statistics.hpp

+ * Based on these, we use combine variables in Polar/Decomposed forms:
+ * - Decomposed: Treat RDV individually as in cartesian co-ordinates with a separate variance for both real and
+ * complex component.
+ * - Polar: RDV is in polar co-ordinates, with magnitude and angle.


This part is a bit hard to flow from the above, perhaps some inbetween liaison?

Suggested change

* Based on these, we use combine variables in Polar/Decomposed forms:

* - Decomposed: Treat RDV individually as in cartesian co-ordinates with a separate variance for both real and

* complex component.

* - Polar: RDV is in polar co-ordinates, with magnitude and angle.

* [liaison]

* Based on these, we use combined variables in Decomposed/Polar forms:

* - Decomposed: treat random variables individually as in cartesian co-ordinates with separated variances for both real and

* imaginary part.

* - Polar: random variables are in polar co-ordinates, with magnitudes and angles.

Jerry-Jinfeng-Guo · 2025-02-19T10:38:00Z

power_grid_model_c/power_grid_model/include/power_grid_model/common/statistics.hpp

+    explicit operator UniformRealRDV<asymmetric_t>() const
+        requires(is_symmetric_v<sym>)
+    {
+        return {.value = RealValue<asymmetric_t>{std::piecewise_construct, value}, .variance = variance};
+    }
+    explicit operator UniformRealRDV<symmetric_t>() const
+        requires(is_asymmetric_v<sym>)
+    {
+        return {.value = mean_val(value), .variance = variance / 3.0};
+    }


Is the division by 3 enough? If I convert one sym rand var to asym rand var and back, would I still get the same result?

Jerry-Jinfeng-Guo · 2025-02-19T10:47:58Z

power_grid_model_c/power_grid_model/include/power_grid_model/common/statistics.hpp

+        return DecomposedComplexRDV<sym>{
+            .real_component = {.value = real_component,
+                               .variance = magnitude.variance * cos_theta * cos_theta +
+                                           imag_component * imag_component * angle.variance},
+            .imag_component = {.value = imag_component,
+                               .variance = magnitude.variance * sin_theta * sin_theta +
+                                           real_component * real_component * angle.variance}};
+    }


Should we add the exact equations in, especially here, to make it easier to follow?

Jerry-Jinfeng-Guo · 2025-02-19T10:49:14Z

power_grid_model_c/power_grid_model/include/power_grid_model/common/statistics.hpp

+
+// Complex measured value of a sensor in p.u. with a uniform variance across all phases and axes of the complex plane
+// (rotationally symmetric)
+template <symmetry_tag sym_type> struct UniformComplexRDV {


Suggested change

template <symmetry_tag sym_type> struct UniformComplexRDV {

template <symmetry_tag sym_type> struct UnifiedComplexRandVar {

Jerry-Jinfeng-Guo · 2025-02-19T10:58:51Z

power_grid_model_c/power_grid_model/include/power_grid_model/common/statistics.hpp

+#pragma once
+
+#include "common.hpp"
+#include "three_phase_tensor.hpp"


this one again ...

Jerry-Jinfeng-Guo

No major comments, except:

Please rename RDV to RandVar
Please rename U(u)niform to U(u)nified
Is it possible to add additional test for the conversion back and forth? I.e., original = Convert_to_a(Convert_to_b(original))
I understant that most (first level) subcases are now adopting the function/struct name directly, is it desirable? I.e., PolarComplexRDV<asymmetric_t> or Polar complex random variable - asymmetric

Jerry-Jinfeng-Guo · 2025-02-19T14:09:48Z

tests/cpp_unit_tests/test_statistics.cpp

+using std::numbers::sqrt2;
+constexpr auto inv_sqrt2 = sqrt2 / 2;


Suggested change

using std::numbers::sqrt2;

constexpr auto inv_sqrt2 = sqrt2 / 2;

constexpr auto inv_sqrt2 = std::numbers::sqrt2 * 0.5;

constexpr auto sqrt3_2 = std::numbers::sqrt3 * 0.5;

Jerry-Jinfeng-Guo · 2025-02-19T14:27:00Z

tests/cpp_unit_tests/test_statistics.cpp

+
+using std::numbers::sqrt2;
+constexpr auto inv_sqrt2 = sqrt2 / 2;
+


These folks are used often enough:

Suggested change

constexpr pi_2 = pi * 0.5;

constexpr pi_3 = pi / 3.0;

constexpr pi_4 = pi * 0.25;

constexpr pi_6 = pi_3 * 0.5;

Jerry-Jinfeng-Guo · 2025-02-19T14:35:15Z

tests/cpp_unit_tests/test_statistics.cpp

+                    CHECK(imag(decomposed.value()) == doctest::Approx(imag(polar.value())));
+                }
+
+                SUBCASE("Perpendicular phase shift") {


Perpendicular would be matching a rotation.

Suggested change

SUBCASE("Perpendicular phase shift") {

SUBCASE("90deg phase shift") {

mgovers and others added 8 commits February 4, 2025 15:56

add statistics module setup

5c8d7b3

Signed-off-by: Martijn Govers <[email protected]>

change to rdv and add some tests

deffcfd

Signed-off-by: Nitish Bharambe <[email protected]>

add decomposed rough

a2fe613

Signed-off-by: Nitish Bharambe <[email protected]>

rename rdv 2

befc6c8

Signed-off-by: Nitish Bharambe <[email protected]>

fix polar tests non conversion

3e20a34

Signed-off-by: Nitish Bharambe <[email protected]>

add remaining tests

72013ec

Signed-off-by: Nitish Bharambe <[email protected]>

wip tests correction

2581a62

Signed-off-by: Nitish Bharambe <[email protected]>

fix remaining

f7cd9eb

Signed-off-by: Nitish Bharambe <[email protected]>

mgovers reviewed Feb 17, 2025

View reviewed changes

nitbharambe added 3 commits February 17, 2025 14:39

cleanup

279e3a9

Signed-off-by: Nitish Bharambe <[email protected]>

structured bindings ref

0012881

Signed-off-by: Nitish Bharambe <[email protected]>

remove irrelavant todos

30f1787

Signed-off-by: Nitish Bharambe <[email protected]>

nitbharambe marked this pull request as ready for review February 17, 2025 14:28

mgovers reviewed Feb 18, 2025

View reviewed changes

add decomposed sym asym conversions

79db492

Signed-off-by: Nitish Bharambe <[email protected]>

nitbharambe requested a review from Jerry-Jinfeng-Guo February 18, 2025 15:32

nitbharambe and others added 2 commits February 18, 2025 16:43

move subcases

101d0a3

Signed-off-by: Nitish Bharambe <[email protected]>

Merge branch 'main' into feature/statistics-module

2cb1b9f

Jerry-Jinfeng-Guo reviewed Feb 19, 2025

View reviewed changes

nitbharambe marked this pull request as draft February 20, 2025 08:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Measurement statistics module #896

Measurement statistics module #896

nitbharambe commented Feb 13, 2025

mgovers Feb 17, 2025

nitbharambe Feb 17, 2025

mgovers Feb 18, 2025

mgovers left a comment

mgovers Feb 18, 2025

nitbharambe commented Feb 18, 2025

sonarqubecloud bot commented Feb 18, 2025

Jerry-Jinfeng-Guo left a comment

Jerry-Jinfeng-Guo Feb 19, 2025

Jerry-Jinfeng-Guo Feb 19, 2025

Jerry-Jinfeng-Guo Feb 19, 2025

Jerry-Jinfeng-Guo Feb 19, 2025

Jerry-Jinfeng-Guo Feb 19, 2025

Jerry-Jinfeng-Guo Feb 19, 2025

Jerry-Jinfeng-Guo Feb 19, 2025

Jerry-Jinfeng-Guo Feb 19, 2025

Jerry-Jinfeng-Guo left a comment

Jerry-Jinfeng-Guo Feb 19, 2025

Jerry-Jinfeng-Guo Feb 19, 2025

Jerry-Jinfeng-Guo Feb 19, 2025

	* randomly distributed variables(RDV) used in the Power Grid Model, like in the State Estimation algorithms to
	* random variables (RandVar) used in the Power Grid Model, like in the State Estimation algorithms to

	* randomly distributed variables(RDV) used in the Power Grid Model, like in the State Estimation algorithms to
	* random variables (RV) used in the Power Grid Model, like in the State Estimation algorithms to

	* A Randomly distributed variable in PGM can have following characteristics:
	* A random variable in PGM can have following characteristics:

- * Based on these, we use combine variables in Polar/Decomposed forms:
- * - Decomposed: Treat RDV individually as in cartesian co-ordinates with a separate variance for both real and
- * complex component.
- * - Polar: RDV is in polar co-ordinates, with magnitude and angle.
+ * [liaison]
+ * Based on these, we use combined variables in Decomposed/Polar forms:
+ * - Decomposed: treat random variables individually as in cartesian co-ordinates with separated variances for both real and
+ * imaginary part.
+ * - Polar: random variables are in polar co-ordinates, with magnitudes and angles.

	template <symmetry_tag sym_type> struct UniformComplexRDV {
	template <symmetry_tag sym_type> struct UnifiedComplexRandVar {

		using std::numbers::sqrt2;
		constexpr auto inv_sqrt2 = sqrt2 / 2;

+    constexpr pi_2 = pi * 0.5;
+    constexpr pi_3 = pi / 3.0;
+    constexpr pi_4 = pi * 0.25;
+    constexpr pi_6 = pi_3 * 0.5;

	SUBCASE("Perpendicular phase shift") {
	SUBCASE("90deg phase shift") {

Measurement statistics module #896

Are you sure you want to change the base?

Measurement statistics module #896

Conversation

nitbharambe commented Feb 13, 2025

Changes proposed in this PR include:

Could you please pay extra attention to the points below when reviewing the PR:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mgovers left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nitbharambe commented Feb 18, 2025

sonarqubecloud bot commented Feb 18, 2025

Quality Gate passed

Jerry-Jinfeng-Guo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jerry-Jinfeng-Guo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment