-
Notifications
You must be signed in to change notification settings - Fork 130
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
Missed time step(s) for noisy / AC current injection in Brian implementation #445
Comments
Added a new global method names 'get_time_epsilon()' in file https://github.com/NeuralEnsemble/PyNN/blob/master/pyNN/utility/__init__.py def get_time_epsilon():
return 1e-10 # 100 picoseconds This is to be imported in the file https://github.com/NeuralEnsemble/PyNN/blob/master/pyNN/brian/standardmodels/electrodes.py from pyNN.utility import get_time_epsilon The test condition for the if statement in _update_current() method here: if self.running and abs(simulator.state.t - self.times[self.i] * 1e3) <= get_time_epsilon(): My original intention was to set the value of EPSILON to an even smaller value of 1e-12. But on using that value, the output for case 1 (above) was like this (see here). Interestingly, the floating point error seemed to progressively increase in magnitude and cross 1e-12 (and subsequently leading to other issues in the above example, as 'self.running' is never set to false). On trying with a larger EPSILON value of 1e-10 things seem to work properly. The output for the earlier examples with this fix are given below:
But if one takes a closer look at those outputs, the value of difference seems to keep rising and for longer simulations could continue to cross even this value of EPSILON. This process is faster when dt is smaller (e.g. 0.01 ms vs 0.1 ms). So a definite fix for this issue might need to consider the following: a) Should we go with a fixed value for EPSILON, or have it variable (relative to values being compared). Having a fixed value is more intuitive, but a relative one might alleviate the above issue of gradual rise in the value of difference. b) It is interesting to note that the instant at which the floating point errors arise is not dependent upon specific timestamps (e.g. 40.1 ms or 50.2 ms), but rather seems to be soon after the initiation of current injection. Could be worth exploring this origin. |
Two factors contributed to the above problem:
Example: import numpy
a = numpy.arange(5, 15, 1)
b = numpy.arange(0.5, 1.5, 0.1) * 10
for i,j in zip(a,b):
print i, "\t", j, "\t", i-j prints:
numpy.arange() basically adds the step-size to the previous value to determine the next value in the sequence. Addition/subtraction of floats leads to such accumulation of errors, whereas multiplication/division do not. Thus, a better approach would be to construct the array by multiplication as shown below. import numpy
a = numpy.arange(5, 15, 1)
c = numpy.array([0.5+(i*0.1) for i in range(10)]) * 10
for i,j in zip(a,c):
print i, "\t", j, "\t", i-j giving:
Note: Floating point values (almost) always involve some rounding-off errors (as seen above for 12), and this discussion is only in context of the accumulation of such (small) errors in leading to a larger error.
a = numpy.arange(1, 1000+1, 1)
b = numpy.arange(0.001, 1+0.001, 0.001)
for i in range(len(a)):
print ("%d\t%0.20f\t%0.20f\t%g" % (i, a[i], b[i]*1000, a[i]-b[i]*1000)) Taking two sample values of the above output corresponding to {a[i]=9, b[i]=0.009} and {a[i]=960, b[i]=0.960}
Quoting from here: http://softwareengineering.stackexchange.com/questions/202843/solutions-for-floating-point-rounding-errors
|
The fix for this matter could be via following changes in https://github.com/NeuralEnsemble/PyNN/blob/master/pyNN/brian/standardmodels/electrodes.py Generate self.times via multiplication (not numpy.arange):This would help eliminate accumulation of floating-point errors. method _generate() for ACSource changed from: def _generate(self):
self.times = numpy.arange(self.start, self.stop, simulator.state.dt) to def _generate(self):
temp_num_t = len(numpy.arange(self.start, self.stop + simulator.state.dt * 1e-3, simulator.state.dt * 1e-3))
self.times = numpy.array([self.start+(i*simulator.state.dt*1e-3) for i in range(temp_num_t)]) and method _generate() for NoisyCurrentSource changed from: def _generate(self):
self.times = numpy.arange(self.start, self.stop, simulator.state.dt) to def _generate(self):
temp_num_t = len(numpy.arange(self.start, self.stop, max(self.dt, simulator.state.dt * 1e-3)))
self.times = numpy.array([self.start+(i*max(self.dt, simulator.state.dt * 1e-3)) for i in range(temp_num_t)])
self.times = numpy.append(self.times, self.stop) Change comparison of timestamps:(Note: the fix suggested here does not require EPSILON, as was suggested in an earlier post) if self.running and simulator.state.t >= self.times[self.i] * 1e3: to if self.running and abs(simulator.state.t - self.times[self.i] * 1e3) < (simulator.state.dt/2.0): The above fix suffices as the maximum time resolution we have during the simulation is simulator.state.dt, and hence any timestamp falling within half this value (on either side) could be considered equal. |
Fixed in #467 |
It was seen that current injection (NoisyCurrentSource and ACSource) failed to invoke the method '_compute()' for a few time intervals. During these intervals (1 or 2 dts) the current injected continued to be that evaluated for the previous time step.
Splitting the if statement in this line: https://github.com/NeuralEnsemble/PyNN/blob/master/pyNN/brian/standardmodels/electrodes.py#L74
and adding a print statement inbetween the two conditions, helps identify this problem. Something like this (continuing from the fix for issue #437):
Snippets from some sample outputs for this code are given below:
Note: s.s.t = simulator.state.t
It can be seen that the issue is due to comparison of floating point numbers, and can be overcome using the concept of EPSILON. This would allows us to set an upper bound on the error due to rounding in floating point arithmetic. Will test the same and update here.
The text was updated successfully, but these errors were encountered: