Cleanup and fixing Izhikevich NGNMM connections

[agchesebro]

No description provided.

[agchesebro]

@helmutstrey I think this should be ready to go for cleaning up the connections. The biggest outstanding thing to do is type the Izhikevich NGNMM blocks to allow for noise, but I'm holding that off until we can discuss noise at the DBS meeting on Monday. Lmk if there's anything else I should clarify in this PR before merging!

Also deprecating a couple out of date neural masses. Checking with Anand before they're removed fully - will include that in Monday's PR.

[hstrey]

Looks great. Why do you call the Izh NGNMM PYR_Izh? Should we not try to be consistent in terms of naming? What are the other NGNMMs based on? I know that the EI is from theta neurons. Maybe we should name them NGNMM_Izh, NGNMM_theta, etc.

[MasonProtter]

What's the reason for commenting out all this old code rather than deleting it?

Also, note that this is a breaking change, since some exported APIs will no longer be usable. Hence, if we do this, we should change the version in the Project.toml to v0.6.0-DEV

[agchesebro]

Looks great. Why do you call the Izh NGNMM PYR_Izh? Should we not try to be consistent in terms of naming? What are the other NGNMMs based on? I know that the EI is from theta neurons. Maybe we should name them NGNMM_Izh, NGNMM_theta, etc.

Yes we should have a common naming convention. It was called that because the original use for this was mimicking the behavior of the Adam's PYR blocks - that's why the QIF ones are QIF_PING because they were a PING function. I think we should unify it, but maybe have a larger discussion first because some of Anand's code will need to change if we mess with the EI theta blocks.

[agchesebro]

What's the reason for commenting out all this old code rather than deleting it?

Also, note that this is a breaking change, since some exported APIs will no longer be usable. Hence, if we do this, we should change the version in the Project.toml to v0.6.0-DEV

Two reasons for commenting - first was I wasn't sure if anything depended on the theta neuron ones I was removing so I was just trying that for the test suite to make sure nothing broke. Second was I was still discussing with Anand if we needed any of them. We confirmed they're not needed conceptually even, so they can be removed. I'll go ahead and delete and update the version.

[agchesebro]

Actually if this is already a breaking change then I'm going to leave this open until we decide the naming conventions for the next-gen blocks, since that will break the APIs again.

[MasonProtter]

Actually if this is already a breaking change then I'm going to leave this open until we decide the naming conventions for the next-gen blocks, since that will break the APIs again.

That's definitely an option, but I would say that we could also just merge it to master, make master the v0.6.0-DEV branch, and then continue to make whatever other breaking changes we might want to make to master. That way this PR doesn't languish for an unknown period of time.

In the meantime, if there are non-breaking changes people want to make and register, we could backport them to a v0.5.x release.

[hstrey]

@agchesebro why is this a breaking change? What is it breaking?

[agchesebro]

@agchesebro why is this a breaking change? What is it breaking?

It's breaking the APIs because I'm removing the old NGNMMs that nobody was using. So it's technically breaking but nobody should notice it with the current changes. It will be more noticeably breaking when we update the naming conventions for the NGNMM blocks, but even then it should be minor even though it's breaking.

[hstrey]

@agchesebro But then I agree with @MasonProtter. Let's change the NGNMM's names to something consistent and create a 0.6.0 release. I would suggest something like NGNMM_Izh, etc. Once we change the NextGenerationEI, we need to update the tutorials and category learning scripts.

[agchesebro]

Sounds good! I'll start switching the names over and pushing some fixes to make sure the tests don't break.

[hstrey]

@agchesebro btw, one trick that we used before not to break code is to declare:
NextGenerationEI = NGNMM_theta
If NGNMM_theta is defined, then the "old" NextGenerationEI is still working.

[agchesebro]

I'll give that a try and see how it goes with the GraphDynamics version of the NextGenerationEI block too.

[agchesebro]

Ok this should be set for review now. Main points so far:

• Names are fixed to uniform naming convention (NGNMM_XXX for different flavors of next-gen masses)
• NGNMM_QIF and NGNMM_Izh support both ODE and SDE via the new typing method
• NextGenerationEIBlox() convenience constructor still allowed so tutorials aren't broken (confirmed with testing hebbian_learning.jl). The GraphDynamics side only supports NGNMM_theta for now, so going forwards we should revise to switch over. Added the deprecation to the docstring.
• Removed a bunch of old source and OUBlox tests that were commented out but not deleted. We've replaced all the ones we've continued supporting so they can be safely deleted.