Skip to content

Tidy DPpropagator class#157

Open
ewanwm wants to merge 5 commits intomainfrom
tidy/refactor-DP-propagator
Open

Tidy DPpropagator class#157
ewanwm wants to merge 5 commits intomainfrom
tidy/refactor-DP-propagator

Conversation

@ewanwm
Copy link
Copy Markdown
Owner

@ewanwm ewanwm commented Sep 1, 2025

No description provided.

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cpp-linter Review

Used clang-tidy v16.0.6

Only 4 out of 8 clang-tidy concerns fit within this pull request's diff.

Click here for the full clang-tidy patch
diff --git a/benchmarks/benchmarks.cpp b/benchmarks/benchmarks.cpp
index dbca5a1..dd007a6 100644
--- a/benchmarks/benchmarks.cpp
+++ b/benchmarks/benchmarks.cpp
@@ -147,2 +147,2 @@ static void BM_DPpropOscillations(benchmark::State &state)
-    DPpropagator dpProp(3, 295000.0, 2.6, 5);
-    
+    DPpropagator dpProp(3, true, 2.6, 5);
+
diff --git a/nuTens/propagator/DP-propagator.cpp b/nuTens/propagator/DP-propagator.cpp
index b978d5e..3a00b7e 100644
--- a/nuTens/propagator/DP-propagator.cpp
+++ b/nuTens/propagator/DP-propagator.cpp
@@ -77,0 +78 @@ Tensor DPpropagator::calculateProbs()
+    {
@@ -80,0 +82 @@ Tensor DPpropagator::calculateProbs()
+    }
diff --git a/nuTens/propagator/DP-propagator.hpp b/nuTens/propagator/DP-propagator.hpp
index 77262b8..da4aac4 100644
--- a/nuTens/propagator/DP-propagator.hpp
+++ b/nuTens/propagator/DP-propagator.hpp
@@ -184 +184 @@ class DPpropagator : public Propagator
-    [[nodiscard]] Tensor calculateProbs();
+    [[nodiscard]] Tensor calculateProbs() override;
diff --git a/python/binding.cpp b/python/binding.cpp
index 15344b5..86a0d83 100644
--- a/python/binding.cpp
+++ b/python/binding.cpp
@@ -443,5 +443,4 @@ void initTesting(py::module &m)
-            std::vector<std::vector<double>> ret = {
-                {probs_returned[0][0], probs_returned[0][1], probs_returned[0][2]},
-                {probs_returned[1][0], probs_returned[1][1], probs_returned[1][2]},
-                {probs_returned[2][0], probs_returned[2][1], probs_returned[2][2]}
-            };
+            std::vector<std::vector<double>> ret =
+                0 = {{probs_returned[0][0], probs_returned[0][1], probs_returned[0][2]},
+                     {probs_returned[1][0], probs_returned[1][1], probs_returned[1][2]},
+                     {probs_returned[2][0], probs_returned[2][1], probs_returned[2][2]}};

Have any feedback or feature suggestions? Share it here.

Tensor B = Tmm + Amatter * See; // B is only needed for N_Newton >= 1
Tensor B = dmsq21 * dmsq31 + Amatter * See; // B is only needed for N_Newton >= 1
for (int i = 0; i < NRiterations; i++)
lambda3 =
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy suggestion

Suggested change
lambda3 =
{

Comment thread nuTens/propagator/DP-propagator.cpp Outdated
Tensor See = Araw + dmsq21 * Ue2sq + dmsq31 * Ue3sq;
Tensor Tee = dmsq21 * dmsq31 * (one - Ue3sq - Ue2sq);

Tensor C = Amatter * Tee;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy diagnostic

nuTens/propagator/DP-propagator.cpp:60:12: warning: [readability-identifier-length]

variable name 'C' is too short, expected at least 3 characters

    Tensor C = Amatter * Tee;
           ^

Comment thread nuTens/propagator/DP-propagator.cpp Outdated

Tensor C = Amatter * Tee;
A = A + Amatter;
Tensor A = Araw + Amatter;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy diagnostic

nuTens/propagator/DP-propagator.cpp:61:12: warning: [readability-identifier-length]

variable name 'A' is too short, expected at least 3 characters

    Tensor A = Araw + Amatter;
           ^
/home/runner/work/nuTens/nuTens/nuTens/propagator/DP-propagator.cpp:71:99: warning: 4.0 is a magic number; consider replacing it with a named constant [cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers]
    Tensor lambda3 = -dmsq31 + Dmsqee * (xmat - 1 + Tensor::pow(tmp * tmp + sinSqTheta13 * xmat * 4.0, 0.5)) * 0.5;
                                                                                                  ^
/home/runner/work/nuTens/nuTens/nuTens/propagator/DP-propagator.cpp:71:104: warning: 0.5 is a magic number; consider replacing it with a named constant [cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers]
    Tensor lambda3 = -dmsq31 + Dmsqee * (xmat - 1 + Tensor::pow(tmp * tmp + sinSqTheta13 * xmat * 4.0, 0.5)) * 0.5;
                                                                                                       ^
/home/runner/work/nuTens/nuTens/nuTens/propagator/DP-propagator.cpp:71:112: warning: 0.5 is a magic number; consider replacing it with a named constant [cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers]
    Tensor lambda3 = -dmsq31 + Dmsqee * (xmat - 1 + Tensor::pow(tmp * tmp + sinSqTheta13 * xmat * 4.0, 0.5)) * 0.5;
                                                                                                               ^

// Newton iterations to improve lambda3 arbitrarily, if needed, (B needed here) //
// ---------------------------------------------------------------------------- //
Tensor B = Tmm + Amatter * See; // B is only needed for N_Newton >= 1
Tensor B = dmsq21 * dmsq31 + Amatter * See; // B is only needed for N_Newton >= 1
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy diagnostic

nuTens/propagator/DP-propagator.cpp:76:12: warning: [readability-identifier-length]

variable name 'B' is too short, expected at least 3 characters

    Tensor B = dmsq21 * dmsq31 + Amatter * See; // B is only needed for N_Newton >= 1
           ^

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Sep 1, 2025

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 91.48936% with 4 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
nuTens/propagator/DP-propagator.cpp 93.33% 1 Missing and 2 partials ⚠️
nuTens/propagator/DP-propagator.hpp 50.00% 0 Missing and 1 partial ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Files with missing lines Coverage Δ
nuTens/propagator/DP-propagator.hpp 66.66% <50.00%> (-8.34%) ⬇️
nuTens/propagator/DP-propagator.cpp 96.25% <93.33%> (-3.75%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Sep 1, 2025

🐰 Bencher Report

Branchtidy/refactor-DP-propagator
Testbedubuntu-latest

⚠️ WARNING: No Threshold found!

Without a Threshold, no Alerts will ever be generated.

Click here to create a new Threshold
For more information, see the Threshold documentation.
To only post results if a Threshold exists, set the --ci-only-thresholds flag.

Click to view all benchmark results
BenchmarkLatencynanoseconds (ns)
Const Density Oscillations/1024/1024📈 view plot
⚠️ NO THRESHOLD
1,916,669,769.00 ns
Const Density Oscillations/1024/1024_cv📈 view plot
⚠️ NO THRESHOLD
0.01 ns
Const Density Oscillations/1024/1024_mean📈 view plot
⚠️ NO THRESHOLD
1,910,082,773.25 ns
Const Density Oscillations/1024/1024_median📈 view plot
⚠️ NO THRESHOLD
1,907,530,153.00 ns
Const Density Oscillations/1024/1024_stddev📈 view plot
⚠️ NO THRESHOLD
9,749,855.38 ns
DP Propagator Const Density Oscillations/1024/1024📈 view plot
⚠️ NO THRESHOLD
963,989,384.00 ns
DP Propagator Const Density Oscillations/1024/1024_cv📈 view plot
⚠️ NO THRESHOLD
0.00 ns
DP Propagator Const Density Oscillations/1024/1024_mean📈 view plot
⚠️ NO THRESHOLD
960,805,631.44 ns
DP Propagator Const Density Oscillations/1024/1024_median📈 view plot
⚠️ NO THRESHOLD
960,856,854.00 ns
DP Propagator Const Density Oscillations/1024/1024_stddev📈 view plot
⚠️ NO THRESHOLD
2,658,678.98 ns
Vacuum Oscillations/1024/1024📈 view plot
⚠️ NO THRESHOLD
408,608,673.00 ns
Vacuum Oscillations/1024/1024_cv📈 view plot
⚠️ NO THRESHOLD
0.01 ns
Vacuum Oscillations/1024/1024_mean📈 view plot
⚠️ NO THRESHOLD
410,820,472.75 ns
Vacuum Oscillations/1024/1024_median📈 view plot
⚠️ NO THRESHOLD
409,453,021.75 ns
Vacuum Oscillations/1024/1024_stddev📈 view plot
⚠️ NO THRESHOLD
5,253,385.17 ns
🐰 View full continuous benchmarking report in Bencher

@github-actions github-actions Bot dismissed their stale review September 11, 2025 09:07

outdated suggestion

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cpp-linter Review

Used clang-tidy v16.0.6

Only 4 out of 8 clang-tidy concerns fit within this pull request's diff.

Click here for the full clang-tidy patch
diff --git a/benchmarks/benchmarks.cpp b/benchmarks/benchmarks.cpp
index dbca5a1..dd007a6 100644
--- a/benchmarks/benchmarks.cpp
+++ b/benchmarks/benchmarks.cpp
@@ -147,2 +147,2 @@ static void BM_DPpropOscillations(benchmark::State &state)
-    DPpropagator dpProp(3, 295000.0, 2.6, 5);
-    
+    DPpropagator dpProp(3, true, 2.6, 5);
+
diff --git a/nuTens/propagator/DP-propagator.cpp b/nuTens/propagator/DP-propagator.cpp
index d744a8c..e2ecb90 100644
--- a/nuTens/propagator/DP-propagator.cpp
+++ b/nuTens/propagator/DP-propagator.cpp
@@ -78,0 +79 @@ Tensor DPpropagator::calculateProbs()
+    {
@@ -81,0 +83 @@ Tensor DPpropagator::calculateProbs()
+    }
diff --git a/nuTens/propagator/DP-propagator.hpp b/nuTens/propagator/DP-propagator.hpp
index 77262b8..da4aac4 100644
--- a/nuTens/propagator/DP-propagator.hpp
+++ b/nuTens/propagator/DP-propagator.hpp
@@ -184 +184 @@ class DPpropagator : public Propagator
-    [[nodiscard]] Tensor calculateProbs();
+    [[nodiscard]] Tensor calculateProbs() override;
diff --git a/python/binding.cpp b/python/binding.cpp
index 15344b5..86a0d83 100644
--- a/python/binding.cpp
+++ b/python/binding.cpp
@@ -443,5 +443,4 @@ void initTesting(py::module &m)
-            std::vector<std::vector<double>> ret = {
-                {probs_returned[0][0], probs_returned[0][1], probs_returned[0][2]},
-                {probs_returned[1][0], probs_returned[1][1], probs_returned[1][2]},
-                {probs_returned[2][0], probs_returned[2][1], probs_returned[2][2]}
-            };
+            std::vector<std::vector<double>> ret =
+                0 = {{probs_returned[0][0], probs_returned[0][1], probs_returned[0][2]},
+                     {probs_returned[1][0], probs_returned[1][1], probs_returned[1][2]},
+                     {probs_returned[2][0], probs_returned[2][1], probs_returned[2][2]}};

Have any feedback or feature suggestions? Share it here.

Tensor B = Tmm + Amatter * See; // B is only needed for N_Newton >= 1
Tensor B = dmsq21 * dmsq31 + Amatter * See; // B is only needed for N_Newton >= 1
for (int i = 0; i < NRiterations; i++)
lambda3 =
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy suggestion

Suggested change
lambda3 =
{

Tensor Tmm = dmsq21 * dmsq31; // using Tmm as a temporary variable
Tensor Tee = Tmm * (one - Ue3sq - Ue2sq);
Tensor Araw = -dmsq21 - dmsq31;
Tensor A = Araw + Amatter;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy diagnostic

nuTens/propagator/DP-propagator.cpp:57:12: warning: [readability-identifier-length]

variable name 'A' is too short, expected at least 3 characters

    Tensor A = Araw + Amatter;
           ^

Tensor Smm = A + dmsq21 * Um2sq + dmsq31 * Um3sq;
Tensor Tmm = dmsq21 * dmsq31 * (one - Um3sq - Um2sq) + Amatter * (See + Smm - A);

Tensor C = Amatter * Tee;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy diagnostic

nuTens/propagator/DP-propagator.cpp:65:12: warning: [readability-identifier-length]

variable name 'C' is too short, expected at least 3 characters

    Tensor C = Amatter * Tee;
           ^
/home/runner/work/nuTens/nuTens/nuTens/propagator/DP-propagator.cpp:72:99: warning: 4.0 is a magic number; consider replacing it with a named constant [cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers]
    Tensor lambda3 = -dmsq31 + Dmsqee * (xmat - 1 + Tensor::pow(tmp * tmp + sinSqTheta13 * xmat * 4.0, 0.5)) * 0.5;
                                                                                                  ^
/home/runner/work/nuTens/nuTens/nuTens/propagator/DP-propagator.cpp:72:104: warning: 0.5 is a magic number; consider replacing it with a named constant [cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers]
    Tensor lambda3 = -dmsq31 + Dmsqee * (xmat - 1 + Tensor::pow(tmp * tmp + sinSqTheta13 * xmat * 4.0, 0.5)) * 0.5;
                                                                                                       ^
/home/runner/work/nuTens/nuTens/nuTens/propagator/DP-propagator.cpp:72:112: warning: 0.5 is a magic number; consider replacing it with a named constant [cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers]
    Tensor lambda3 = -dmsq31 + Dmsqee * (xmat - 1 + Tensor::pow(tmp * tmp + sinSqTheta13 * xmat * 4.0, 0.5)) * 0.5;
                                                                                                               ^

// Newton iterations to improve lambda3 arbitrarily, if needed, (B needed here) //
// ---------------------------------------------------------------------------- //
Tensor B = Tmm + Amatter * See; // B is only needed for N_Newton >= 1
Tensor B = dmsq21 * dmsq31 + Amatter * See; // B is only needed for N_Newton >= 1
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy diagnostic

nuTens/propagator/DP-propagator.cpp:77:12: warning: [readability-identifier-length]

variable name 'B' is too short, expected at least 3 characters

    Tensor B = dmsq21 * dmsq31 + Amatter * See; // B is only needed for N_Newton >= 1
           ^

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants