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

Add rectified Gabor distribution for nest.spatial #2387

Merged
merged 21 commits into from
Aug 30, 2023

Conversation

ackurth
Copy link
Contributor

@ackurth ackurth commented May 6, 2022

A new nest.spatial_distributions functions to generate a spatial connectivity profile with probabilities described by a Gabor function (see here).
While in principle it seems to be possible to obtain the same functionality with other build in nest functions, this would, to my understanding, be rather cumbersome. Therefore I think that it might be worthwhile to add this function to nest on this level.

Note that the parametrization used for the two-dimensional Gaussian differs from the one of nest.spatial_distributions.gaussian2D.
I think the parametrization used in this PR is more intuitive and could be used also for this function (see #2388).

@heplesser heplesser added T: Enhancement New functionality, model or documentation S: Normal Handle this with default priority I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) labels May 6, 2022
@heplesser heplesser added this to PRs in progress in Kernel via automation May 6, 2022
Copy link
Contributor

@heplesser heplesser left a comment

Choose a reason for hiding this comment

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

@ackurth Thank you for this addition. Given the wide use of Gabors it does seem sensible to have support for them at the C++-level. Could you maybe also add an example? See #2388 (comment) concerning the parameterization.

Kernel automation moved this from PRs in progress to PRs pending approval May 6, 2022
@heplesser
Copy link
Contributor

I suggest we wait a bit with this one until we have discussed #2391.

@github-actions
Copy link

Pull request automatically marked stale!

@github-actions github-actions bot added the stale Automatic marker for inactivity, please have another look here label Jul 13, 2022
@terhorstd terhorstd added the P: Blocked Work on this can not continue, see comments for the particular reason label Jan 16, 2023
@hakonsbm hakonsbm removed their request for review January 27, 2023 13:10
@med-ayssar
Copy link
Contributor

@heplesser & @ackurth any news regarding this PR and the issue #2391

@github-actions github-actions bot removed the stale Automatic marker for inactivity, please have another look here label Apr 7, 2023
@med-ayssar
Copy link
Contributor

@heplesser & @ackurth any news regarding this PR?

Copy link
Contributor

@heplesser heplesser left a comment

Choose a reason for hiding this comment

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

@ackurth I approve this now, #2391 will have to wait. Could you merge master to allow merging this after a second reviewer has commented?

@heplesser heplesser requested a review from JoseJVS August 7, 2023 13:58
@heplesser heplesser removed the P: Blocked Work on this can not continue, see comments for the particular reason label Aug 7, 2023
Copy link
Contributor

@JoseJVS JoseJVS left a comment

Choose a reason for hiding this comment

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

Looks good.

@heplesser
Copy link
Contributor

@ackurth Could you check the remaining code-formatting issues?

nestkernel/parameter.cpp Outdated Show resolved Hide resolved
nestkernel/parameter.cpp Outdated Show resolved Hide resolved
nestkernel/parameter.h Outdated Show resolved Hide resolved
nestkernel/parameter.cpp Outdated Show resolved Hide resolved
nestkernel/parameter.cpp Outdated Show resolved Hide resolved
nestkernel/parameter.cpp Outdated Show resolved Hide resolved
terhorstd and others added 2 commits August 28, 2023 13:06
Co-authored-by: Jose Villamar <62692462+JoseJVS@users.noreply.github.com>
nestkernel/parameter.cpp Outdated Show resolved Hide resolved
@heplesser heplesser merged commit c73c297 into nest:master Aug 30, 2023
20 checks passed
Kernel automation moved this from PRs pending approval to Done Aug 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) S: Normal Handle this with default priority T: Enhancement New functionality, model or documentation
Projects
Kernel
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants