-
Notifications
You must be signed in to change notification settings - Fork 18
[issue-779] Populate STDP edge recorder output #949
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: LikithaDev
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -93,11 +93,36 @@ void ConnStatic::printParameters() const | |
| void ConnStatic::registerHistoryVariables() | ||
| { | ||
| // Register the following variables to be recorded | ||
| // Note: There may be potential duplicate weight, source, destination vertices | ||
| Recorder &recorder = Simulator::getInstance().getModel().getRecorder(); | ||
| recorder.registerVariable("weight", WCurrentEpoch_, Recorder::UpdatedType::DYNAMIC); | ||
| recorder.registerVariable("sourceVertex", sourceVertexIndexCurrentEpoch_, | ||
| Recorder::UpdatedType::DYNAMIC); | ||
| recorder.registerVariable("destinationVertex", destVertexIndexCurrentEpoch_, | ||
| Recorder::UpdatedType::DYNAMIC); | ||
| } | ||
|
|
||
| bool ConnStatic::updateConnections() | ||
| { | ||
| // GPU STDP is not implemented, so this only uses the CPU edge data. | ||
| WCurrentEpoch_.startNewEpoch(); | ||
| sourceVertexIndexCurrentEpoch_.startNewEpoch(); | ||
| destVertexIndexCurrentEpoch_.startNewEpoch(); | ||
|
Comment on lines
+107
to
+109
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right now, these are called in the |
||
|
|
||
| AllEdges &edges = getEdges(); | ||
| const vector<unsigned char> &inUse = edges.getInUse(); | ||
| const vector<BGFLOAT> &weights = edges.getWeights(); | ||
| const vector<int> &sourceVertices = edges.getSourceVertexIndices(); | ||
| const vector<int> &destVertices = edges.getDestVertexIndices(); | ||
|
|
||
| // Copy one value per active edge into parallel recorder vectors. | ||
| // Entries at the same index describe the same edge. | ||
| for (BGSIZE iEdg = 0; iEdg < inUse.size(); iEdg++) { | ||
| if (inUse[iEdg]) { | ||
| WCurrentEpoch_.push_back(weights[iEdg]); | ||
| sourceVertexIndexCurrentEpoch_.push_back(sourceVertices[iEdg]); | ||
| destVertexIndexCurrentEpoch_.push_back(destVertices[iEdg]); | ||
|
Comment on lines
+121
to
+123
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Two questions:
Use the CoPilot unit test skill to generate unit tests for this. It should catch the case where these aren't cleared and so aren't copies. Actually, can't you just use the assignment operator,
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should reply and resolve this, as appropriate, to finalize this review. |
||
| } | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| /** | ||
| * @file ConnStaticTests.cpp | ||
| * | ||
| * @brief Unit tests for ConnStatic. | ||
| * | ||
| * @ingroup Testing/UnitTesting | ||
| */ | ||
|
|
||
| #include "AllSTDPSynapses.h" | ||
| #include "ConnStatic.h" | ||
| #include "gtest/gtest.h" | ||
| #include <memory> | ||
| #include <vector> | ||
|
|
||
| namespace { | ||
|
|
||
| class TestConnStatic : public ConnStatic { | ||
| public: | ||
| void setEdges(std::unique_ptr<AllEdges> edges) | ||
| { | ||
| edges_ = std::move(edges); | ||
| } | ||
| }; | ||
|
|
||
| TEST(ConnStatic, UpdateConnectionsCopiesActiveEdgesWithoutAccumulating) | ||
| { | ||
| auto edges = std::make_unique<AllSTDPSynapses>(3, 2); | ||
| edges->addEdge(edgeType::EE, 0, 2, 0.001); | ||
| edges->addEdge(edgeType::II, 2, 0, 0.001); | ||
|
|
||
| const std::vector<int> expectedSources {2, 0}; | ||
| const std::vector<int> expectedDestinations {0, 2}; | ||
| const std::vector<BGFLOAT> expectedWeights {-10.0e-9, 10.0e-9}; | ||
|
|
||
| TestConnStatic connections; | ||
| connections.setEdges(std::move(edges)); | ||
|
|
||
| connections.updateConnections(); | ||
|
|
||
| EXPECT_EQ(expectedSources, connections.getSourceVertexIndexCurrentEpoch()); | ||
| EXPECT_EQ(expectedDestinations, connections.getDestVertexIndexCurrentEpoch()); | ||
| EXPECT_EQ(expectedWeights, connections.getWCurrentEpoch()); | ||
|
|
||
| connections.updateConnections(); | ||
|
|
||
| EXPECT_EQ(expectedSources, connections.getSourceVertexIndexCurrentEpoch()); | ||
| EXPECT_EQ(expectedDestinations, connections.getDestVertexIndexCurrentEpoch()); | ||
| EXPECT_EQ(expectedWeights, connections.getWCurrentEpoch()); | ||
| } | ||
|
|
||
| } // namespace |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In particular, see my questions attached to the
UpdateConnections()method. A unit test should be able to verify that theConnStaticdata members are indeed copies of those from the Edge class.