Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

recording file symantics #449

Closed
alan-stokes opened this issue Feb 17, 2017 · 1 comment
Closed

recording file symantics #449

alan-stokes opened this issue Feb 17, 2017 · 1 comment
Labels
Milestone

Comments

@alan-stokes
Copy link

Hi all,

still looking into the recording interface for getting the spinnaker version of PyNN 0.8 one up and running, and saw what may be a semantic implication mismatch.

when you say record, your adding new atoms to be recorded (grand)
but your also able to add a outputfile (tofile=None). This tofile parameter if filled is sent to the recording interface on its own (with no context to which variable is being recorded). Therefore it overrides all previous set tofiles, as its just a basestring, not say a tuple or dict nor a function call with the variable name attached. so the following calls results in what id believe end users would say is unexpected behavior

pop.record("spikes", tofile="spikes.txt")
this sets spikes to go to spikes.txt
pop.record('v', tofile="v.txt")
this sets spikes and v to go to v,.txt
whereas as a end user, id expect spikes to go to spikes.txt and v to go to v.txt

the reasoning why i think this behavior is going to happen is from here:
https://github.com/NeuralEnsemble/PyNN/blob/master/pyNN/common/populations.py#L439-L440
as this sets the file for the recorder, but without any context to which variable its being recorded for and is the only place where tofile is handed down to the recorder interface.

regards
Alan

@apdavison apdavison added this to the future milestone Mar 8, 2017
@apdavison
Copy link
Member

Hi Alan,

Your expectation as an end user is correct; this is an implementation bug. I think the reason it hasn't come to light earlier is that normally you would only call population.record() once, e.g.

pop.record(["spikes", "v"], to_file="data.h5")

Andrew

apdavison added a commit that referenced this issue Jun 30, 2017
Fixes #449, #490, #491: Corrects simulator and population recording
pgleeson added a commit to pgleeson/PyNN that referenced this issue Aug 4, 2017
* master: (66 commits)
  Update Travis
  Fixes NeuralEnsemble#497: tweaks phase for NEST ACSource (NeuralEnsemble#502)
  Implement `Grid3D.generate_positions()` for `fill_order="random"` (fix for NeuralEnsemble#504)
  Fixes issue NeuralEnsemble#499: updates sim.end() for Brian
  Updates NeuralEnsemble#459: Updates docstring of find_units()
  Updates NeuralEnsemble#459: Adds docstring to find_units()
  Fixes 459: Extends find_units() for neuron parameters
  Removes recording sub-test for mock - needs reimplementation with pyNN.mock
  Fix a test that fails with NumPy 1.13
  Temporarily install quantities from github, due to python-quantities/python-quantities#129
  Still trying to fix NEST build with Python 2.7 on Travis
  Updates NeuralEnsemble#490, NeuralEnsemble#491; minor changes
  It seems a recent NumPy change made it an error to treat a single-element array as a scalar for the purposes of indexing.
  Numpy became much more strict about certain arguments being int not float
  Travis updated to Python 2.7.13 (need to find a way to avoid hard-coding this in the NEST install script)
  Updates NeuralEnsemble#490 (also fixes NeuralEnsemble#449): handles recording without file
  Fixes issue NeuralEnsemble#490: moves write_on_end to populations.py
  Fixes 491: updates recording to handle individual cells as PopulationView
  Updates fix for issue NeuralEnsemble#487 - handles boundary condition
  Fixes issue NeuralEnsemble#487: Updates _update_iclamp() for NEURON
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants