Skip to content

[ISSUE-7, 10] Moved all model-specific defaults from Global.h to appropriate model classes#928

Open
CodersRepo wants to merge 11 commits intoSharedDevelopmentfrom
DhruvaDev
Open

[ISSUE-7, 10] Moved all model-specific defaults from Global.h to appropriate model classes#928
CodersRepo wants to merge 11 commits intoSharedDevelopmentfrom
DhruvaDev

Conversation

@CodersRepo
Copy link
Copy Markdown

Closes #7 #10

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

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the simulator’s neuroscience models to remove model-specific default constants from Simulator/Utils/Global.h and place them into the owning neuron/synapse classes, improving encapsulation and addressing inconsistencies noted in #7 and #10.

Changes:

  • Moved IF neuron default parameters from Global.h into AllIFNeurons (static constexpr).
  • Moved spiking synapse defaults (tau, U, delay weight) into AllSpikingSynapses and updated host/GPU code paths to reference them.
  • Removed the corresponding #define DEFAULT_* macros from Global.h.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
Simulator/Vertices/Neuro/AllIFNeurons.h Adds static constexpr IF neuron defaults to the model class.
Simulator/Vertices/Neuro/AllIFNeurons.cpp Switches neuron initialization to use AllIFNeurons::DEFAULT_* values.
Simulator/Utils/Global.h Removes model-specific default macros from the global header.
Simulator/Edges/Neuro/AllSynapsesDeviceFuncs_d.cpp Updates CUDA device-side synapse creation defaults to use AllSpikingSynapses::DEFAULT_* and adds required include.
Simulator/Edges/Neuro/AllSpikingSynapses.h Introduces static constexpr defaults for spiking synapses.
Simulator/Edges/Neuro/AllSpikingSynapses.cpp Uses the new AllSpikingSynapses::DEFAULT_tau in edge creation.
Simulator/Edges/Neuro/AllDynamicSTDPSynapses.cpp Replaces DEFAULT_U usage with AllSpikingSynapses::DEFAULT_U.
Simulator/Edges/Neuro/AllDSSynapses.cpp Replaces DEFAULT_U usage with AllSpikingSynapses::DEFAULT_U.
Contributors.md Adds a 2026 entry.
Comments suppressed due to low confidence (2)

Simulator/Edges/Neuro/AllSynapsesDeviceFuncs_d.cpp:120

  • In createSpikingSynapse(), the switch(type) has a default: break; and then tau/delay are used to compute decay/totalDelay. If an unexpected edgeType reaches here, this will use uninitialized values (UB) on the device. Consider asserting in the default case (like the CPU path does) or initializing tau/delay to safe defaults before the switch.
   allEdgesDevice->tau_[iEdg] = AllSpikingSynapses::DEFAULT_tau;

   BGFLOAT tau;
   switch (type) {
      case edgeType::II:

Simulator/Edges/Neuro/AllSynapsesDeviceFuncs_d.cpp:267

  • In createSTDPSynapse(), the switch(type) has a default: break; but tau/delay are used immediately afterwards to compute decay/totalDelay. If type is outside {II, IE, EI, EE}, this will use uninitialized values on the device. Recommend asserting in the default branch or setting tau/delay defaults before the switch.
   allEdgesDevice->tau_[iEdg] = AllSpikingSynapses::DEFAULT_tau;

   BGFLOAT tau;
   switch (type) {
      case edgeType::II:

static constexpr BGFLOAT DEFAULT_InhibTrefract
= 2.0e-3; // The default absolute refractory period for inhibitory neurons
static constexpr BGFLOAT DEFAULT_ExcitTrefract
= 3.0e-3; // The default absolute refractory period for excitory neurons
Comment on lines +181 to 185
allEdgesDevice->U_[iEdg] = AllSpikingSynapses::DEFAULT_U;
allEdgesDevice->tau_[iEdg] = AllSpikingSynapses::DEFAULT_tau;

BGFLOAT U;
BGFLOAT D;
Comment on lines 346 to +350
allEdgesDevice->lastSpike_[iEdg] = ULONG_MAX;
allEdgesDevice->type_[iEdg] = type;

allEdgesDevice->U_[iEdg] = DEFAULT_U;
allEdgesDevice->tau_[iEdg] = DEFAULT_tau;
allEdgesDevice->U_[iEdg] = AllSpikingSynapses::DEFAULT_U;
allEdgesDevice->tau_[iEdg] = AllSpikingSynapses::DEFAULT_tau;
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