Skip to content

Conversation

@NicolasJPosey
Copy link
Contributor

@NicolasJPosey NicolasJPosey commented Mar 7, 2025

Closes #786 #806

Description

Checklist (Mandatory for new features)

  • Added Documentation
  • Added Unit Tests

Testing (Mandatory for all changes)

  • GPU Test: test-medium-connected.xml Passed
  • GPU Test: test-large-long.xml Passed

NicolasJPosey and others added 5 commits February 3, 2025 15:32
* Refactor model class to generalize the summation logic method and move implementation into neuron class

* Implement new AllVertices method in 911 class

* Fix last compile errors

* Clean up commented out code

* Resolve code format failure

* Remove dynamic cast of vertices so model with work with non-neuron models

* Add override keyword to integrateVertexInputs declaration

* Fix formatting issue
* Rename edge index map device variable name

* Rename method names to remove neuro context

* Rename variables both in method signature and implementation

* Fix formatting

* Remove neuro from GPU model and generalize what we can

* Remove summation point operation as it's now done in the neuro vertices base class

* Revert formatting that breaks code style

* Clean up comments, variable names, and includes

* Move edge sum index into all spiking synapses and rename neuro GPU methods in both edges and vertices

* Fix style violation

* Make corresponding updates to documentation

* Re add serialize method to documentation

* Fix const representation

* Add All911Edges to connections documentation

* Update diagram images
Moved small 911 test into Cpu subfolder since it was generated with Cpu code. This required corresponding update to github actions. Then updated RunTests so that it would build the appropriate list of tests based on the current processing unit.
* Implement integrateVertexInputs in All911Vertices

Since advanceEdges doesn't update the edge state until after we know that we can transfer the call, we move all of the advanceEdges logic into integrateVertexInputs.

* fix style issue
@NicolasJPosey NicolasJPosey added refactor doesn't change functionality, just improves code NG911 testing labels Mar 7, 2025
@NicolasJPosey NicolasJPosey self-assigned this Mar 7, 2025
@NicolasJPosey NicolasJPosey marked this pull request as ready for review March 7, 2025 08:05
@NicolasJPosey NicolasJPosey requested a review from stiber March 7, 2025 08:05
@NicolasJPosey
Copy link
Contributor Author

@stiber changes are mostly the same as the two PRs that we reviewed. Had a merge conflict with a change Diva made to fix a seg fault that I resolved, but it doesn't look like it's in the list of changes. One weird thing I noticed was that Raiju is causing some of the GPU regression tests to fail while Otachi is not. I thought I remember us running into that last year when I was setting up the GPU test outputs. I think that is also the last time I used Raiju for GPU stuff. Not sure if that is something worth us looking into or not.

Copy link
Contributor

@stiber stiber left a comment

Choose a reason for hiding this comment

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

You'll probably will need to merge SharedDevelopment into this first.

@NicolasJPosey NicolasJPosey merged commit 481e1ce into SharedDevelopment Mar 13, 2025
2 checks passed
@NicolasJPosey NicolasJPosey deleted the PoseyDevelopment branch March 13, 2025 23:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

NG911 refactor doesn't change functionality, just improves code testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants