Skip to content
This repository was archived by the owner on Jun 11, 2025. It is now read-only.

feat: Add a Spike Recorder device#27

Open
filimarc wants to merge 16 commits intodevfrom
feature/spike_recorder
Open

feat: Add a Spike Recorder device#27
filimarc wants to merge 16 commits intodevfrom
feature/spike_recorder

Conversation

@filimarc
Copy link
Contributor

@filimarc filimarc commented Feb 26, 2025

A new device for spike detection is proposed. It cycle over the cells in the population, assigning an gid to every location to record.
SpikeTrains for cells in the same population are joined togheter, i tried to emulate the SpikeTrains annotations of the nest case. The default locations is "soma".

  • Add tests

@github-actions github-actions bot added the feat label Feb 26, 2025
@filimarc
Copy link
Contributor Author

filimarc commented Feb 26, 2025

In device/spike_recorder.py
I will need to use Location instead of LocationAccessor so i do:

locations = [
                    k
                    for k, v in target.locations.items()
                    if v in self.locations.get_locations(target)
]

But maybe there is a better way to do that.

I have also to think a sort of test for this device...

@danilobenozzo
Copy link
Contributor

Try with
for location in self.locations.get_locations(target):
location._loc

@filimarc filimarc requested a review from Helveg March 3, 2025 10:36
@drodarie
Copy link
Contributor

drodarie commented Apr 1, 2025

@Helveg could you please review this PR, we would like to bump the main bsb package.

Copy link
Contributor

@Helveg Helveg left a comment

Choose a reason for hiding this comment

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

Couple of small issues, but I think this is good :)


def check_netcon(self, adapter, target, location):
# Insert a NetCon (if not already present) and retrieve its gid
if hasattr(location.section, "_transmitter"):
Copy link
Contributor

Choose a reason for hiding this comment

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

This is an implementation detail of arborize, this private attribute should not be accessed directly in the BSB, instead the proper util function of arborize should be used/added to retrieve this

Copy link
Contributor Author

@filimarc filimarc May 19, 2025

Choose a reason for hiding this comment

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

I moved the check_netcon method to arborize

adapter, simulation, simdata
).items():
if self.join_population:
spike_times = p.parallel._interpreter.Vector()
Copy link
Contributor

Choose a reason for hiding this comment

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

this doesn't look like the proper idiom for accessing the hoc interpreter either

Copy link
Contributor Author

@filimarc filimarc May 19, 2025

Choose a reason for hiding this comment

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

Since i haven't found a better way to access the interpreter Vector i have created a property in
ParallelContext for that. If you think this is a good approach i will create the PR on Arborize and Patch.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, it's been too long since I designed the bsb-neuron code, so I don't remember, but we'd have to look through the source code to answer these questions:

  • Does the NeuronAdapter ever use the hoc interpreter directly?
    • If so, how does it do it? We should stick to a single pattern and repeat it.
    • If it hasn't ever used the interpreter directly, why would we have to now? Perhaps, by design, I have always avoided this, and used utility functions in the underlying packages? If so, we should probably also repeat that pattern now.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that models are free to build themselves however they like, and that there isn't much interface you can rely on. I think most code so far has been written assuming every model will be built by the arborize package. We'd have to have a meeting to talk about this decision, and write out documentation. Without a decision it's hard to create a specification/interface that the adapter can rely on for targetting/devices, which require the interface to retrieve information about the model instances.

Copy link
Contributor Author

@filimarc filimarc May 19, 2025

Choose a reason for hiding this comment

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

Sorry i have already understood that i can call directly without any modification of patch:
spike_times = p.Vector()
To answer to your question in BSB-Neuron devices usually we do not use directly the Hoc Interpreter, but we call it in the record function in NeuronResult. The difference is that in this new case we have to create a Vector shared among different cells.

@filimarc filimarc changed the base branch from main to dev May 19, 2025 07:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants