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

Issue with DCSource in Brian when value changing values on the fly #451

Closed
appukuttan-shailesh opened this issue Mar 1, 2017 · 3 comments
Milestone

Comments

@appukuttan-shailesh
Copy link
Contributor

The code here attempts to change the values of the DCSource on the fly. The amplitude is 0 for the first 100 ms, and thereafter changed for t > 100 ms. But as the start and stop times are specified less than 100 ms, there should be no effect on the membrane potential. As seen below the output in Brian shows an erroneous value at the transition.

Brian output (incorrect):
brian_bad
NEURON output (correct):
neuron_good

@appukuttan-shailesh
Copy link
Contributor Author

appukuttan-shailesh commented Mar 15, 2017

This can be rectified by incorporating the below mentioned changes in https://github.com/NeuralEnsemble/PyNN/blob/master/pyNN/brian/standardmodels/electrodes.py.
There are two main changes:

  1. the index (self.i) to the time and amplitudes list is reset to zero only at the start of a new run, and not for continuation of existing runs (which can involve changes on the fly). This involves changing:
    def _reset(self):
        self.i = 0
        self.running = True
        if self._is_computed:
            self._generate()

to

    def _reset(self):
        if not hasattr(self, 'running') or self.running == False:
            self.i = 0
            self.running = True
        if self._is_computed:
            self._generate()
  1. Adding the current simulation time and corresponding amplitude in the times (self.times) and amplitudes (self.amplitudes) list, while ensuring that the temporal ordering is maintained. This is achieved by modifying the _generate() method for DCSource from:
    def _generate(self):
        if self.start == 0:
            self.times = [self.start, self.stop]
            self.amplitudes = [self.amplitude, 0.0]
        else:
            self.times = [0.0, self.start, self.stop]
            self.amplitudes = [0.0, self.amplitude, 0.0]

to

    def _generate(self):
        if self.start == 0:
            self.times = [self.start, self.stop]
            self.amplitudes = [self.amplitude, 0.0]
        else:
            self.times = [0.0, self.start, self.stop]
            self.amplitudes = [0.0, self.amplitude, 0.0]
        if self.start < simulator.state.t*1e-3 < self.stop:
            self.times.insert(-1, simulator.state.t*1e-3)
            self.amplitudes.insert(-1, self.amplitude)
            if (self.start==0 and self.i==2) or (self.start!=0 and self.i==3):
                self.i -= 1

These changes were tested against various start/stop combos for the DCSource described in this file. The earlier test file was not used owing to problems arising from issue #452 .

Just for the record, the Brian output after this fix (in context to figures in the initial post):
brian_fixed
Note: This seems to resolve the issue of changing parameters on the fly for the DCSource. It would be useful to have this checked for the other electrode classes.

@apdavison
Copy link
Member

@appukuttan-shailesh thanks for investigating this. Can you create a pull request with these changes (and a test), please?

@apdavison
Copy link
Member

Fixed in #467

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants