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

Make neural mass housekeeping PR non-breaking #532

Merged

Conversation

MasonProtter
Copy link
Contributor

This is a PR that is targeting #530, rather than a commit to that branch because I want to see what people think of this. This is an idea I had to make that PR non-breaking.

Basically, instead of having two separate types for noisy versus non-noisy, this PR puts them together into one type, but they differ by a type parameter, e.g.

julia> typeof(KuramotoOscillator(;name=:n, include_noise=true))
KuramotoOscillator{Neuroblox.Noisy}

julia> typeof(KuramotoOscillator(;name=:n, include_noise=false))
KuramotoOscillator{Neuroblox.NonNoisy}

I also changed the keyword argument back to include_noise rather than noise, since that is what current Neuroblox master uses.

Any thoughts or feelings on this approach?

@harisorgn
Copy link
Member

I prefer this, tidier type tree, as in theory we could apply it to all atomic Blox.

@agchesebro
Copy link
Member

Oh yeah I think this is much better. Seems easily extended to have delay types as well, and avoids the messiness of those constructor functions.

@MasonProtter
Copy link
Contributor Author

Great, I'll merge it into your PR then.

@MasonProtter MasonProtter merged commit 1786741 into neural-mass-housekeeping Jan 17, 2025
1 check passed
@MasonProtter MasonProtter deleted the nonbreaking-neural-mass-housekeeping branch January 17, 2025 16:16
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.

3 participants