Feature instrumentation profiling #35

ewanwm · 2024-08-01T14:16:44Z

Add in new instrumentation library.

Currently supports instrumented code profiling:
If built with -DNT_PROFILING=ON then tests will output a -profile.json that can be placed into chrome tracing to get something like

…ptions for the project a bit

codecov-commenter · 2024-08-01T14:20:40Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Files	Coverage Δ
nuTens/propagator/const-density-solver.cpp	`100.00% <ø> (ø)`
nuTens/propagator/const-density-solver.hpp	`92.30% <100.00%> (ø)`
nuTens/propagator/propagator.cpp	`72.22% <ø> (ø)`
nuTens/propagator/propagator.hpp	`42.85% <ø> (ø)`
nuTens/tensors/tensor.hpp	`87.50% <ø> (ø)`
nuTens/tensors/torch-tensor.cpp	`64.19% <ø> (ø)`
tests/tensor-basic.cpp	`92.98% <ø> (ø)`
tests/two-flavour-const-matter.cpp	`100.00% <ø> (ø)`
tests/two-flavour-vacuum.cpp	`100.00% <ø> (ø)`

github-actions

Cpp-linter Review

No concerns from clang-format.
Only 13 out of 15 clang-tidy concerns fit within this pull request's diff.

Click here for the full clang-tidy patch

diff --git a/nuTens/instrumentation.hpp b/nuTens/instrumentation.hpp
index f302e6c..6bab99e 100644
--- a/nuTens/instrumentation.hpp
+++ b/nuTens/instrumentation.hpp
@@ -7,0 +8 @@
+#include <utility>
@@ -48 +49 @@ class ProfileWriter
-    ProfileWriter() : _name(""), _profileCount(0)
+    ProfileWriter()  
@@ -84 +85 @@ class ProfileWriter
-        _outputStream << "\"cat\":\"function\",";
+        _outputStream << R"("cat":"function",)";
@@ -86,2 +87,2 @@ class ProfileWriter
-        _outputStream << "\"name\":\"" << name << "\",";
-        _outputStream << "\"ph\":\"X\",";
+        _outputStream << R"("name":")" << name << "\",";
+        _outputStream << R"("ph":"X",)";
@@ -99 +100 @@ class ProfileWriter
-        _outputStream << "{\"otherData\": {},\"traceEvents\":[";
+        _outputStream << R"({"otherData": {},"traceEvents":[)";
@@ -121 +122 @@ class ProfileWriter
-    uint _profileCount;
+    uint _profileCount{0};
@@ -137 +138 @@ class InstrumentationTimer
-    InstrumentationTimer(const std::string &name) : _name(name), _stopped(false)
+    InstrumentationTimer(std::string name) : _name(std::move(name)), _stopped(false)
@@ -145 +146 @@ class InstrumentationTimer
-        if (!_stopped)
+        if (!_stopped) {
@@ -146,0 +148 @@ class InstrumentationTimer
+}
diff --git a/nuTens/propagator/const-density-solver.hpp b/nuTens/propagator/const-density-solver.hpp
index 26c87ee..e275f02 100644
--- a/nuTens/propagator/const-density-solver.hpp
+++ b/nuTens/propagator/const-density-solver.hpp
@@ -45 +45 @@ class ConstDensityMatterSolver : public BaseMatterSolver
-    inline void setPMNS(const Tensor &newPMNS)
+    inline void setPMNS(const Tensor &newPMNS) override
@@ -58 +58 @@ class ConstDensityMatterSolver : public BaseMatterSolver
-    inline void setMasses(const Tensor &newMasses)
+    inline void setMasses(const Tensor &newMasses) override
@@ -81 +81 @@ class ConstDensityMatterSolver : public BaseMatterSolver
-    void calculateEigenvalues(const Tensor &energies, Tensor &eigenvectors, Tensor &eigenvalues);
+    void calculateEigenvalues(const Tensor &energies, Tensor &eigenvectors, Tensor &eigenvalues) override;

Have any feedback or feature suggestions? Share it here.

nuTens/instrumentation.hpp

github-actions · 2024-08-01T14:28:45Z

nuTens/instrumentation.hpp

+    uint _profileCount;
+};
+
+class InstrumentationTimer


clang-tidy diagnostic

nuTens/instrumentation.hpp:124:7: warning: [cppcoreguidelines-special-member-functions]

class 'InstrumentationTimer' defines a non-default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator

class InstrumentationTimer ^

nuTens/propagator/const-density-solver.hpp

github-actions · 2024-08-01T14:28:45Z

nuTens/propagator/propagator.hpp

@@ -91,6 +94,7 @@ class Propagator
    /// @param value The new value
    inline void setPMNS(const std::vector<int> &indices, std::complex<float> value)


clang-tidy diagnostic

nuTens/propagator/propagator.hpp:95:2: warning: [clang-diagnostic-delete-abstract-non-virtual-dtor]

delete called on 'BaseMatterSolver' that is abstract but has non-virtual destructor

delete __ptr; ^ /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/unique_ptr.h:396:4: note: in instantiation of member function 'std::default_delete<BaseMatterSolver>::operator()' requested here get_deleter()(std::move(__ptr)); ^ /home/runner/work/nuTens/nuTens/nuTens/propagator/propagator.hpp:30:5: note: in instantiation of member function 'std::unique_ptr<BaseMatterSolver>::~unique_ptr' requested here Propagator(int nGenerations, float baseline) : _baseline(baseline), _nGenerations(nGenerations){}; ^

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

outdated suggestion

github-actions

Cpp-linter Review

Click here for the full clang-format patch

diff --git a/nuTens/instrumentation.hpp b/nuTens/instrumentation.hpp
index b9127e3..fb0746a 100644
--- a/nuTens/instrumentation.hpp
+++ b/nuTens/instrumentation.hpp
@@ -48 +48 @@ class ProfileWriter
-    ProfileWriter()  
+    ProfileWriter()
@@ -145 +145,2 @@ class InstrumentationTimer
-        if (!_stopped) {
+        if (!_stopped)
+        {
@@ -146,0 +148 @@ class InstrumentationTimer
+        }
@@ -148 +149,0 @@ class InstrumentationTimer
-}

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

Click here for the full clang-tidy patch

diff --git a/nuTens/instrumentation.hpp b/nuTens/instrumentation.hpp
index b9127e3..d5dda3d 100644
--- a/nuTens/instrumentation.hpp
+++ b/nuTens/instrumentation.hpp
@@ -49,2 +49 @@ class ProfileWriter
-    {
-    }
+    = default;
diff --git a/nuTens/propagator/const-density-solver.hpp b/nuTens/propagator/const-density-solver.hpp
index 3ff1e48..e275f02 100644
--- a/nuTens/propagator/const-density-solver.hpp
+++ b/nuTens/propagator/const-density-solver.hpp
@@ -81 +81 @@ class ConstDensityMatterSolver : public BaseMatterSolver
-    void calculateEigenvalues(const Tensor &energies, Tensor &eigenvectors, Tensor &eigenvalues);
+    void calculateEigenvalues(const Tensor &energies, Tensor &eigenvectors, Tensor &eigenvalues) override;

Have any feedback or feature suggestions? Share it here.

nuTens/instrumentation.hpp

github-actions · 2024-08-01T14:57:50Z

nuTens/instrumentation.hpp

+        if (!_stopped) {
+            stop();
+    }
+}


clang-format suggestions

Please remove the line(s)

148

nuTens/instrumentation.hpp

github-actions · 2024-08-01T14:57:50Z

nuTens/instrumentation.hpp

+    uint _profileCount{0};
+};
+
+class InstrumentationTimer


clang-tidy diagnostic

nuTens/instrumentation.hpp:124:7: warning: [cppcoreguidelines-special-member-functions]

class 'InstrumentationTimer' defines a non-default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator

class InstrumentationTimer ^

github-actions · 2024-08-01T14:57:50Z

nuTens/propagator/propagator.hpp

@@ -91,6 +94,7 @@ class Propagator
    /// @param value The new value
    inline void setPMNS(const std::vector<int> &indices, std::complex<float> value)


clang-tidy diagnostic

nuTens/propagator/propagator.hpp:95:2: warning: [clang-diagnostic-delete-abstract-non-virtual-dtor]

delete called on 'BaseMatterSolver' that is abstract but has non-virtual destructor

delete __ptr; ^ /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/unique_ptr.h:396:4: note: in instantiation of member function 'std::default_delete<BaseMatterSolver>::operator()' requested here get_deleter()(std::move(__ptr)); ^ /home/runner/work/nuTens/nuTens/nuTens/propagator/propagator.hpp:30:5: note: in instantiation of member function 'std::unique_ptr<BaseMatterSolver>::~unique_ptr' requested here Propagator(int nGenerations, float baseline) : _baseline(baseline), _nGenerations(nGenerations){}; ^

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

outdated suggestion

github-actions

Cpp-linter Review

Click here for the full clang-format patch

diff --git a/nuTens/instrumentation.hpp b/nuTens/instrumentation.hpp
index a42c9ac..494e941 100644
--- a/nuTens/instrumentation.hpp
+++ b/nuTens/instrumentation.hpp
@@ -48,2 +48 @@ class ProfileWriter
-    ProfileWriter()
-    = default;
+    ProfileWriter() = default;
@@ -147 +145,0 @@ class InstrumentationTimer
-    }
@@ -148,0 +147 @@ class InstrumentationTimer
+    }

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

Click here for the full clang-tidy patch

diff --git a/nuTens/propagator/const-density-solver.hpp b/nuTens/propagator/const-density-solver.hpp
index 3ff1e48..e275f02 100644
--- a/nuTens/propagator/const-density-solver.hpp
+++ b/nuTens/propagator/const-density-solver.hpp
@@ -81 +81 @@ class ConstDensityMatterSolver : public BaseMatterSolver
-    void calculateEigenvalues(const Tensor &energies, Tensor &eigenvectors, Tensor &eigenvalues);
+    void calculateEigenvalues(const Tensor &energies, Tensor &eigenvectors, Tensor &eigenvalues) override;

Have any feedback or feature suggestions? Share it here.

github-actions · 2024-08-01T15:12:46Z

nuTens/instrumentation.hpp

+    ProfileWriter()
+    = default;


clang-format suggestions

Suggested change

ProfileWriter()

= default;

ProfileWriter() = default;

nuTens/instrumentation.hpp

github-actions · 2024-08-01T15:12:47Z

nuTens/instrumentation.hpp

+    uint _profileCount{0};
+};
+
+class InstrumentationTimer


clang-tidy diagnostic

nuTens/instrumentation.hpp:123:7: warning: [cppcoreguidelines-special-member-functions]

class 'InstrumentationTimer' defines a non-default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator

class InstrumentationTimer ^

github-actions · 2024-08-01T15:12:47Z

nuTens/propagator/propagator.hpp

@@ -91,6 +94,7 @@ class Propagator
    /// @param value The new value
    inline void setPMNS(const std::vector<int> &indices, std::complex<float> value)


clang-tidy diagnostic

nuTens/propagator/propagator.hpp:95:2: warning: [clang-diagnostic-delete-abstract-non-virtual-dtor]

delete called on 'BaseMatterSolver' that is abstract but has non-virtual destructor

delete __ptr; ^ /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/unique_ptr.h:396:4: note: in instantiation of member function 'std::default_delete<BaseMatterSolver>::operator()' requested here get_deleter()(std::move(__ptr)); ^ /home/runner/work/nuTens/nuTens/nuTens/propagator/propagator.hpp:30:5: note: in instantiation of member function 'std::unique_ptr<BaseMatterSolver>::~unique_ptr' requested here Propagator(int nGenerations, float baseline) : _baseline(baseline), _nGenerations(nGenerations){}; ^

github-actions · 2024-08-01T15:48:48Z

Cpp-Linter Report ⚠️

Some files did not pass the configured checks!

clang-tidy reports: 2 concern(s)

nuTens/instrumentation.hpp:122:7: warning: [cppcoreguidelines-special-member-functions]

class 'InstrumentationTimer' defines a non-default destructor and a copy constructor but does not define a copy assignment operator, a move constructor or a move assignment operator
```
class InstrumentationTimer
      ^
```

nuTens/propagator/const-density-solver.hpp:81:10: warning: [clang-diagnostic-inconsistent-missing-override]

'calculateEigenvalues' overrides a member function but is not marked 'override'

    void calculateEigenvalues(const Tensor &energies, Tensor &eigenvectors, Tensor &eigenvalues);
         ^
/home/runner/work/nuTens/nuTens/nuTens/propagator/base-matter-solver.hpp:18:18: note: overridden virtual function is here
    virtual void calculateEigenvalues(const Tensor &energies, Tensor &eigenvectors, Tensor &eigenvalues) = 0;
                 ^
/home/runner/work/nuTens/nuTens/nuTens/propagator/const-density-solver.hpp:81:10: warning: annotate this function with 'override' or (rarely) 'final' [cppcoreguidelines-explicit-virtual-functions,modernize-use-override]
    void calculateEigenvalues(const Tensor &energies, Tensor &eigenvectors, Tensor &eigenvalues);
         ^
                                                                                                 override

Have any feedback or feature suggestions? Share it here.

outdated suggestion

github-actions

Cpp-linter Review

No concerns from clang-format.
Only 2 out of 4 clang-tidy concerns fit within this pull request's diff.

Click here for the full clang-tidy patch

diff --git a/nuTens/propagator/const-density-solver.hpp b/nuTens/propagator/const-density-solver.hpp
index 3ff1e48..e275f02 100644
--- a/nuTens/propagator/const-density-solver.hpp
+++ b/nuTens/propagator/const-density-solver.hpp
@@ -81 +81 @@ class ConstDensityMatterSolver : public BaseMatterSolver
-    void calculateEigenvalues(const Tensor &energies, Tensor &eigenvectors, Tensor &eigenvalues);
+    void calculateEigenvalues(const Tensor &energies, Tensor &eigenvectors, Tensor &eigenvalues) override;

Have any feedback or feature suggestions? Share it here.

github-actions · 2024-08-01T15:48:50Z

nuTens/instrumentation.hpp

+    uint _profileCount{0};
+};
+
+class InstrumentationTimer


clang-tidy diagnostic

nuTens/instrumentation.hpp:122:7: warning: [cppcoreguidelines-special-member-functions]

class 'InstrumentationTimer' defines a non-default destructor and a copy constructor but does not define a copy assignment operator, a move constructor or a move assignment operator

class InstrumentationTimer ^

github-actions · 2024-08-01T15:48:50Z

nuTens/propagator/propagator.hpp

@@ -91,6 +94,7 @@ class Propagator
    /// @param value The new value
    inline void setPMNS(const std::vector<int> &indices, std::complex<float> value)


clang-tidy diagnostic

nuTens/propagator/propagator.hpp:95:2: warning: [clang-diagnostic-delete-abstract-non-virtual-dtor]

delete called on 'BaseMatterSolver' that is abstract but has non-virtual destructor

delete __ptr; ^ /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/unique_ptr.h:396:4: note: in instantiation of member function 'std::default_delete<BaseMatterSolver>::operator()' requested here get_deleter()(std::move(__ptr)); ^ /home/runner/work/nuTens/nuTens/nuTens/propagator/propagator.hpp:30:5: note: in instantiation of member function 'std::unique_ptr<BaseMatterSolver>::~unique_ptr' requested here Propagator(int nGenerations, float baseline) : _baseline(baseline), _nGenerations(nGenerations){}; ^

ewanwm added 7 commits August 1, 2024 06:14

add instrumentation header and set up CMake. Also tidy up the cmake o…

b90b87e

…ptions for the project a bit

add functionality to do instrumented profiling

a8862b7

integrate profiling into Tensor

8733c8b

add instrumentation to propagators

96d7685

add start and stop for instrumentation sessions to the test apps

4f6da3d

Update README.md

b3cb6b8

better name for profile results

336f480

github-actions bot previously requested changes Aug 1, 2024

View reviewed changes

Apply suggestions from code review

7cb2f90

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

github-actions bot previously requested changes Aug 1, 2024

View reviewed changes

Apply suggestions from code review

5f978a6

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

github-actions bot previously requested changes Aug 1, 2024

View reviewed changes

delete the copy constructor for InstrumentationTimer

26b3dd6

github-actions bot requested changes Aug 1, 2024

View reviewed changes

ewanwm merged commit 671de1e into main Aug 1, 2024
1 of 2 checks passed

ewanwm deleted the feature_instrumentation_profiling branch August 1, 2024 15:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature instrumentation profiling #35

Feature instrumentation profiling #35

ewanwm commented Aug 1, 2024

codecov-commenter commented Aug 1, 2024 •

edited

Loading

github-actions bot left a comment

github-actions bot Aug 1, 2024

github-actions bot Aug 1, 2024

github-actions bot left a comment

github-actions bot Aug 1, 2024

github-actions bot Aug 1, 2024

github-actions bot Aug 1, 2024

github-actions bot left a comment

github-actions bot Aug 1, 2024

github-actions bot Aug 1, 2024

github-actions bot Aug 1, 2024

github-actions bot commented Aug 1, 2024

github-actions bot left a comment

github-actions bot Aug 1, 2024

github-actions bot Aug 1, 2024

		@@ -91,6 +94,7 @@ class Propagator
		/// @param value The new value
		inline void setPMNS(const std::vector<int> &indices, std::complex<float> value)

Feature instrumentation profiling #35

Feature instrumentation profiling #35

Conversation

ewanwm commented Aug 1, 2024

codecov-commenter commented Aug 1, 2024 • edited Loading

Codecov Report

github-actions bot left a comment

Choose a reason for hiding this comment

Cpp-linter Review

github-actions bot Aug 1, 2024

Choose a reason for hiding this comment

clang-tidy diagnostic

github-actions bot Aug 1, 2024

Choose a reason for hiding this comment

clang-tidy diagnostic

github-actions bot left a comment

Choose a reason for hiding this comment

Cpp-linter Review

github-actions bot Aug 1, 2024

Choose a reason for hiding this comment

clang-format suggestions

github-actions bot Aug 1, 2024

Choose a reason for hiding this comment

clang-tidy diagnostic

github-actions bot Aug 1, 2024

Choose a reason for hiding this comment

clang-tidy diagnostic

github-actions bot left a comment

Choose a reason for hiding this comment

Cpp-linter Review

github-actions bot Aug 1, 2024

Choose a reason for hiding this comment

clang-format suggestions

github-actions bot Aug 1, 2024

Choose a reason for hiding this comment

clang-tidy diagnostic

github-actions bot Aug 1, 2024

Choose a reason for hiding this comment

clang-tidy diagnostic

github-actions bot commented Aug 1, 2024

Cpp-Linter Report ⚠️

github-actions bot left a comment

Choose a reason for hiding this comment

Cpp-linter Review

github-actions bot Aug 1, 2024

Choose a reason for hiding this comment

clang-tidy diagnostic

github-actions bot Aug 1, 2024

Choose a reason for hiding this comment

clang-tidy diagnostic

codecov-commenter commented Aug 1, 2024 •

edited

Loading