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

GSoC Example #39

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

GSoC Example #39

wants to merge 1 commit into from

Conversation

pb663
Copy link

@pb663 pb663 commented Mar 23, 2021

Fitting multiple sets of parameters to the adaptive exponential IF model from Brette and Gerstner (2005). Example for GSoC project.

Fitting multiple sets of parameters to the adaptive exponential IF model from Brette and Gerstner (2005). Example for GSoC project.
Copy link
Member

@mstimberg mstimberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the example, looks very good to me in general. I made a few minor comments in the code, and it would be great to have a few more comments that explain how the three parameter regimes are handled (e.g. the shape of the list of lists, the duplicated parameter values in the beginning when you generate the ground truth data, etc.) – this might not be obvious to every user.

metric = GammaFactor(delta=1*ms, time=140*ms)

fitters = []
for i in range(len(out_spikes)):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be something like

for spikes in out_spikes:

instead.

Comment on lines +61 to +63
#Put spike spikes and voltage traces into lists , probably lots of smarter ways to do this.
out_spikes = [[spike_train[0] / second, spike_train[1] /second], [spike_train[2] / second, spike_train[3] / second], [spike_train[4] / second, spike_train[5] / second]]
v_traces = [[trace.vm[0]/mV, trace.vm[1]/mV], [trace.vm[2]/mV, trace.vm[3]/mV], [trace.vm[4]/mV, trace.vm[5]/mV]]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is ok as it is. There are "smarter" ways to do this, but they are also less clear. I'd probably change the formatting a bit, though. E.e. have spike_train[0], spike_train[1] on one line, and then spike_train[2], spike_train[3] on the next, etc.
Also, no need to divide by seconds here, and I'd rather keep the v_traces with the units as well (for more consistency in the plotting code).

out_spikes = [[spike_train[0] / second, spike_train[1] /second], [spike_train[2] / second, spike_train[3] / second], [spike_train[4] / second, spike_train[5] / second]]
v_traces = [[trace.vm[0]/mV, trace.vm[1]/mV], [trace.vm[2]/mV, trace.vm[3]/mV], [trace.vm[4]/mV, trace.vm[5]/mV]]

start_scope()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be necessary here.

Comment on lines +124 to +127
ax[0, i].plot(trace.t/ms, v_traces[i][0], label='Simulated');
ax[0, i].plot(trace.t/ms, fits[i][0]/mV, label='Fitted');
ax[1, i].plot(trace.t/ms, v_traces[i][1]);
ax[1, i].plot(trace.t/ms, fits[i][1]/mV);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for semicola here.

ax[0, 0].set_xlabel('Time (ms)')
ax[0, 0].set_ylabel('v (mV)')

plt.show()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Getting really nit-picky: please add a line break in the end :)

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

Successfully merging this pull request may close these issues.

None yet

2 participants