jonescompneurolab / hnn-core

Simulation and optimization of neural circuits for MEG/EEG source estimates
https://jonescompneurolab.github.io/hnn-core/
BSD 3-Clause "New" or "Revised" License
50 stars 50 forks source link

rename simulate_dipole -> simulate #669

Open rythorpe opened 10 months ago

rythorpe commented 10 months ago

I've been thinking about this for some time now: how do we feel about renaming the simulate_dipole() function? Maybe this is a bit nitpicky, but I feel that a function called simulate() in network.py or network_builder.py would better suit the various dynamics HNN-core is now simulating (i.e., not just current dipoles). If we decide to entertain such an idea, it'd be better to make this API change sooner than later and to think critically about what the new function will return. In keeping with the roots of HNN, we can still emphasize that each simulation always returns, at the very least, a dipole. However, there's probably a more intuitive way to refactor how/where users interact with the outputs of a simulation: list of Dipole, CellResponse, and dict of ExtracellularArray. E.g.,

dpl, cell_response, rec_arrays = simulate(net, tstop=170.0)

where dpl is an instance of Dipole, cell_response is an instance of CellResponse, and rec_arrays is an instance of ExtracellularArray. Each of the output objects should be converted to have a similar structure for handling multiple trials and multiple subcomponents (e.g., Dipole has cells from different layers that contribute to the aggregate current dipole while CellResponse has multiple cells that can fire). Furthermore, CellResponse will be detached from Network and provided with any network-specific attributes (e.g., gid_ranges) that allow for reading from and writing to disk.

jasmainak commented 10 months ago

The alternative is simulate(net) or net.simulate() and have net.dipole, net.cell_response and net.rec_arrays as attributes ? The advantage is that you can add more output data in the future if needed.

rythorpe commented 10 months ago

I was imagining leaving these things as detached from Network as possible so that it's more intuitive for users to separate the model from its outputs. Totally open to other opinions on this, however.

ntolley commented 10 months ago

I agree with separating the model from the outputs in principle, but I think the last time we talked about this the model is somewhat inextricable from the outputs (thinking of vsec/isec where the recordings are only meaningful when you know a lot about the model).

The way other simulators achieve separation is more like:

net  = jones_2009_mode()
dpl = net.record_dipole()
cell_response = net.record_cell_respone()

simulate(net)

Not a huge fan of this approach because your variables get modified in functions that don't explicitly involve them.