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

Fixes #497: tweaks phase for NEST ACSource #502

Merged
merged 7 commits into from
Jul 17, 2017

Conversation

appukuttan-shailesh
Copy link
Contributor

@appukuttan-shailesh appukuttan-shailesh commented Jul 6, 2017

Fixes #497 (see for details)
Tweaks the value of phase supplied to NEST ACSource so as to remain consistent with other simulators. The test case for this fix requires the recording of current profiles. As the final current recording implementation is currently unavailable in the 'master' branch, the test has been excluded for the time being. Tested locally to ensure it works.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.09%) to 42.319% when pulling c71381a on appukuttan-shailesh:fix_497 into b340a9e on NeuralEnsemble:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.07%) to 42.302% when pulling e38d4b1 on appukuttan-shailesh:fix_497 into b340a9e on NeuralEnsemble:master.

@coveralls
Copy link

coveralls commented Jul 6, 2017

Coverage Status

Coverage increased (+11.9%) to 54.131% when pulling 073d61e on appukuttan-shailesh:fix_497 into b340a9e on NeuralEnsemble:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.09%) to 42.319% when pulling d228963 on appukuttan-shailesh:fix_497 into b340a9e on NeuralEnsemble:master.

@coveralls
Copy link

coveralls commented Jul 6, 2017

Coverage Status

Coverage increased (+11.9%) to 54.142% when pulling 18b4db0 on appukuttan-shailesh:fix_497 into b340a9e on NeuralEnsemble:master.

@appukuttan-shailesh
Copy link
Contributor Author

phase_fix depends upon frequency and start time (and also specified phase). These parameters are always set by pyNN.nest before setting the phase, hence no issues of order of parameters specified by user. E.g. the following would be equivalent for phase_fix:

acsource1 = ACSource(start=5.0, stop=20.0, amplitude=amplitude, offset=0.0, frequency=100.0, phase=0.0)

and

acsource1 = ACSource(stop=20.0, amplitude=amplitude, offset=0.0, phase=0.0, start=5.0, frequency=100.0)

But the problem arises if the user specified a different start time or frequency subsequently. This needs to be handled in this fix. Will update once this is done, with appropriate changes in the test.

@appukuttan-shailesh
Copy link
Contributor Author

Fix (and test) updated to handle the above. Looks good for now.

@coveralls
Copy link

coveralls commented Jul 7, 2017

Coverage Status

Coverage increased (+11.9%) to 54.175% when pulling 041e700 on appukuttan-shailesh:fix_497 into b340a9e on NeuralEnsemble:master.

@apdavison apdavison merged commit 39d01bb into NeuralEnsemble:master Jul 17, 2017
@apdavison apdavison added this to the 0.9.2 milestone Jul 17, 2017
pgleeson added a commit to pgleeson/PyNN that referenced this pull request 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
  ...
@appukuttan-shailesh appukuttan-shailesh deleted the fix_497 branch August 30, 2017 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Difference in implementation of AC source in NEST
3 participants