Conversation
The logic for loading from sonata files is to swap out the list of parameters if the BBP TYPE is not present Likewise, when instantiating a synapse object, if no TYPE field is present, assume it is for allen and create a Exp2Syn synapse
…e instantiated. The reporting adds try/except since these don't have a soma
… target point neurons and deliver replay spikes to the proper netcon
Removed call to finalize for point process type cells since this is not needed
Updated allen tests to collection cell_info and get ExpSyn data as consequence
…re netcon code Remove more unused code sections; can be added when use case example is available
Added additional test for invalid Mechanism given to HocPointProcessCell
darshanmandge
left a comment
darshanmandge
left a comment
There was a problem hiding this comment.
Thank you, @jamesgking, for adding this to bluecellulab! I have left some comments.
| logger = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| class BasePointProcessCell(Cell): |
There was a problem hiding this comment.
This inherits from Cell but never calls super().__init__(), so none of Cell's attributes (self.recordings, self.soma, self.cell, self.record_dt, etc.) are initialized. This means inherited methods like get_time(), get_recording(), etc., will crash with AttributeError. The bare except Exception: continue blocks added in utils.py may catch the errors though. Maybe initialising the minimum required attributes so inherited methods degrade gracefully instead of relying on catch-all exception handlers.
| vs.play(vec) | ||
|
|
||
| if self.pointcell is None: | ||
| raise ValueError("attempting to add replay spikes with valid pointprocess") |
There was a problem hiding this comment.
Should the message be ... without valid pointprocess"?
| if not hasattr(self, "_replay_vecs"): | ||
| self._replay_vecs: list[h.Vector] = [] | ||
| if not hasattr(self, "_replay_vecstims"): | ||
| self._replay_vecstims: list[h.VecStim] = [] | ||
| if not hasattr(self, "_replay_netcons"): | ||
| self._replay_netcons: list[h.NetCon] = [] |
There was a problem hiding this comment.
Could _replay_vecs, _replay_vecstims and _replay_netcon be initialised in init() instead?
| point_params = PointProcessConnParameters(syn_description[SynapseProperty.PRE_GID], syn_description[SynapseProperty.PRE_GID], | ||
| syn_description[SynapseProperty.AXONAL_DELAY]) |
There was a problem hiding this comment.
The PointProcessConnParameters constructor is called as PointProcessConnParameters(PRE_GID, PRE_GID, AXONAL_DELAY). The dataclass fields are (sgid, delay, weight). So delay gets PRE_GID, and weight gets AXONAL_DELAY. Looks like a bug. Likely should be something like PointProcessConnParameters(sgid=PRE_GID, delay=AXONAL_DELAY, weight=conductance).
| point_params = PointProcessConnParameters(syn_description[SynapseProperty.PRE_GID], syn_description[SynapseProperty.PRE_GID], | ||
| syn_description[SynapseProperty.AXONAL_DELAY]) | ||
|
|
||
| self.pointConn = PointProcessConnection([point_params]) |
There was a problem hiding this comment.
This is assigned but never stored in self.connections or used anywhere else. The connection object is created and then effectively discarded on the next call. Is this intentional?
|
|
||
| for cell_id, cell in cells.items(): | ||
| sec = cell.soma | ||
| try: |
There was a problem hiding this comment.
The catch-all try block silently skips cells that lack a soma. This can hide real errors in morphological cells. Maybe isinstance(cell, BasePointProcessCell) to skip point cells explicitly would be better and let other errors propagate?
| cell = cells.get(cell_id) | ||
| if cell is None: | ||
| continue | ||
| try: |
There was a problem hiding this comment.
Same as above, except the catch-all try block silently skips cells that lack a get_time() method.
| PRE_GID = "pre_gid" | ||
| AXONAL_DELAY = "axonal_delay" | ||
| POST_SECTION_ID = "post_section_id" | ||
| POST_SECTION_POS = "post_section_pos" |
There was a problem hiding this comment.
POST_SECTION_POS was added here but never referenced anywhere in code. Should it be removed?
| SIM_DIR = Path(__file__).parent.parent.absolute() / "examples" / "ringtest_allen_v1" | ||
|
|
||
|
|
||
| def test_cell_point_create(capsys): |
There was a problem hiding this comment.
In all 4 test functions, the capsys parameter is declared but never used. Can it be removed?
| assert (threshold_vals[cell_id.id] == cell.threshold) | ||
|
|
||
|
|
||
| def test_point_process_cell_rejects_none(capsys): |
There was a problem hiding this comment.
Both in lines 98-105 and 107-115: These tests silently pass if the exception is NOT raised. You can instead use pytest.raises(ValueError, match=...) and pytest.raises(BluecellulabError, match=...).
3 participants
This PR adds support for SONATA point-process cells.
Point-process cells are instantiated from SONATA model_type = "point_process" and model_template, mapping directly to NEURON point mechanisms defined in HOC/MOD files.
What is included