Skip to content

Commit

Permalink
[BBPBGLIB-1076] Make all edge managers set load_offsets at init (#69)
Browse files Browse the repository at this point in the history
## Context
Even with a small execution we were hitting
```
323 NEURON: gid=1000000 already exists on this process as an output port
```
When outputting some debug it turned out that the post synaptic edge
index was not being taken into account for the NGV netcon id. Therefore
the id collision was almost certain.

## The problem

Turns out the `load_offset` property was not always set to True in the
connection manager.

This likely happens since in newer Sonata circuits there might be no
internal connectivity, where such setting was being applied.

## Solution

With the current readers we always have synapse ids. So we could
generalize the concept and set
 - All managers to load their synapse indices (ctor arg)
- All except NGV which will never require them, so we explicitly
disabled.
 - Setting applied even for connection managers holding projections only

## Testing

Tested with Katta's small NGV circuit.

## Review
* [x] PR description is complete
* [x] Coding style (imports, function length, New functions, classes or
files) are good
* [ ] Unit/Scientific test added
* [ ] Updated Readme, in-code, developer documentation
  • Loading branch information
ferdonline authored Oct 31, 2023
1 parent da68e6c commit 43c9714
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 12 deletions.
10 changes: 4 additions & 6 deletions neurodamus/connection_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -271,9 +271,9 @@ def __init__(self, circuit_conf, target_manager, cell_manager, src_cell_manager=
self._raw_gids = cell_manager.local_nodes.raw_gids()
self._total_connections = 0
self.circuit_conf = circuit_conf
self._load_offsets = False
self._src_target_filter = None # filter by src target in all_connect (E.g: GapJ)

# Load offsets might be required globally. Conn managers shall honor this if possible
self._load_offsets = kw.get("load_offsets", False)
# An internal var to enable collection of synapse statistics to a Counter
self._dry_run_stats: DryRunStats = kw.get("dry_run_stats")

Expand All @@ -288,15 +288,14 @@ def open_edge_location(self, syn_source, circuit_conf, **kw):
src_pop_id = _get_projection_population_id(circuit_conf)
return self.open_synapse_file(edge_file, pop_name, n_files, src_pop_id=src_pop_id, **kw)

def open_synapse_file(self, synapse_file, edge_population, n_files=1, load_offsets=False, *,
def open_synapse_file(self, synapse_file, edge_population, n_files=1, *,
src_pop_id=None, src_name=None, **_kw):
"""Initializes a reader for Synapses config objects and associated population
Args:
synapse_file: The nrn/edge file. For old nrn files it may be a dir.
edge_population: The population of the edges
n_files: (nrn only) the number of nrn files in the directory (without nrn.h5)
load_offsets: Whether the synapse offset should be loaded. So far only for NGV
src_pop_id: (compat) Allow overriding the src population ID
src_name: The source pop name, normally matching that of the source cell manager
"""
Expand All @@ -311,8 +310,7 @@ def open_synapse_file(self, synapse_file, edge_population, n_files=1, load_offse
n_files = 1

self._synapse_reader = self._open_synapse_file(synapse_file, edge_population, n_files)
self._load_offsets = load_offsets
if load_offsets:
if self._load_offsets:
if not self._synapse_reader.has_property("synapse_index"):
raise Exception("Synapse offsets required but not available. "
"Please use a more recent version of neurodamus-core/synapse-tool")
Expand Down
7 changes: 2 additions & 5 deletions neurodamus/morphio_wrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -209,11 +209,8 @@ def _build_morph(self):
'A H5 file morphology is not supposed to have a soma of type: {}'.format(
self._morph.soma_type))
logging.debug(
"({}, {}, {}) has soma type : {}",
self._collection_dir,
self._morph_name,
self._morph_ext,
self._morph.soma_type
"(%s, %s, %s) has soma type : %s",
self._collection_dir, self._morph_name, self._morph_ext, self._morph.soma_type
)

if self._morph.soma_type == SomaType.SOMA_SINGLE_POINT:
Expand Down
4 changes: 4 additions & 0 deletions neurodamus/ngv.py
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,10 @@ class NeuroGliaConnManager(ConnectionManagerBase):
conn_factory = NeuroGlialConnection
SynapseReader = NeuroGlialSynapseReader

def __init__(self, circuit_conf, target_manager, cell_manager, src_cell_manager=None, **kw):
kw.pop("load_offsets")
super().__init__(circuit_conf, target_manager, cell_manager, src_cell_manager, **kw)

def _add_synapses(self, cur_conn, syns_params, syn_type_restrict=None, base_id=0):
for syn_params in syns_params:
cur_conn.add_synapse(None, syn_params)
Expand Down
2 changes: 1 addition & 1 deletion neurodamus/node.py
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,7 @@ def create_synapses(self):

log_stage("Handling projections...")
for pname, projection in SimConfig.projections.items():
self._load_projections(pname, projection, dry_run_stats=self._dry_run_stats)
self._load_projections(pname, projection, **manager_kwa)

if SimConfig.dry_run:
self.syn_total_memory = self._dry_run_stats.collect_display_syn_counts()
Expand Down

0 comments on commit 43c9714

Please sign in to comment.