-
Notifications
You must be signed in to change notification settings - Fork 92
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
[Enh] add example and handle one-dimensional arrays in CSD.generate lfp #594
Merged
Moritz-Alexander-Kern
merged 13 commits into
NeuralEnsemble:master
from
INM-6:enh/generate_lfp
Oct 24, 2023
Merged
Changes from 7 commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
a6b8f84
add example to documentation
f33fb64
convert string formatting to f-strings
6343b9c
add handling for 1-dimensional arrays
210f27b
add regression test, testing for one- and two-dimensional arrays
ee4501d
typo
c0f8ff1
add TODO for docstring
97732eb
fix doctests for generate lfp
60b6b47
Update elephant/test/test_current_source_density.py
mdenker ade17e8
Update elephant/test/test_current_source_density.py
mdenker 546c5c6
add refenrecen to neo Analogsignal in estimate_csd docstring
d7ba512
Merge remote-tracking branch 'origin/enh/generate_lfp' into enh/gener…
fe7f9ec
specified expected dimensionality for x-y-z positions array in docstring
15b2ed6
Merge branch 'master' into enh/generate_lfp
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -79,8 +79,8 @@ def estimate_csd(lfp, coordinates='coordinates', method=None, | |||
with dimension of space, where M is the number of signals in 'lfp', | ||||
and N is equal to the dimensionality of the method. | ||||
Alternatively, if coordinates is a string, the function will fetch the | ||||
coordinates, supplied in the same format, as annotation of 'lfp' by that | ||||
name. | ||||
coordinates, supplied in the same format, as annotation of 'lfp' by | ||||
that name. | ||||
Default: 'coordinates' | ||||
method : string | ||||
Pick a method corresponding to the setup, in this implementation | ||||
|
@@ -182,14 +182,14 @@ def estimate_csd(lfp, coordinates='coordinates', method=None, | |||
# All iCSD methods explicitly assume a source | ||||
# diameter in contrast to the stdCSD that | ||||
# implicitly assume infinite source radius | ||||
raise ValueError("Parameter diam must be specified for iCSD \ | ||||
methods: {}".format(", ".join(icsd_methods))) | ||||
raise ValueError(f"Parameter diam must be specified for iCSD " | ||||
f"methods: {', '.join(icsd_methods)}") | ||||
|
||||
if 'f_type' in kwargs: | ||||
if (kwargs['f_type'] != 'identity') and \ | ||||
(kwargs['f_order'] is None): | ||||
raise ValueError("The order of {} filter must be \ | ||||
specified".format(kwargs['f_type'])) | ||||
raise ValueError(f"The order of {kwargs['f_type']} filter must" | ||||
f" be specified") | ||||
|
||||
csd_method = getattr(icsd, method) # fetch class from icsd.py file | ||||
csd_estimator = csd_method(lfp=lfp.T.magnitude * lfp.units, | ||||
|
@@ -227,7 +227,7 @@ def generate_lfp(csd_profile, x_positions, y_positions=None, z_positions=None, | |||
1D : gauss_1d_dipole | ||||
2D : large_source_2D and small_source_2D | ||||
3D : gauss_3d_dipole | ||||
x_positions : np.ndarray | ||||
x_positions : np.ndarray # TODO: add dimensionality for x-y-z_positions | ||||
Positions of the x coordinates of the electrodes | ||||
y_positions : np.ndarray, optional | ||||
Positions of the y coordinates of the electrodes | ||||
|
@@ -253,10 +253,31 @@ def generate_lfp(csd_profile, x_positions, y_positions=None, z_positions=None, | |||
|
||||
Returns | ||||
------- | ||||
LFP : neo.AnalogSignal | ||||
LFP : :class:`neo.core.AnalogSignal` | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change should also apply in line 74, right?
(Can't directly suggest the change here, because it's too far from any lines changed in this PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indeed, thanks for catching this, changed accordingly. |
||||
The potentials created by the csd profile at the electrode positions. | ||||
The electrode positions are attached as an annotation named | ||||
'coordinates'. | ||||
|
||||
Examples | ||||
-------- | ||||
>>> import numpy as np | ||||
>>> from elephant.current_source_density import generate_lfp, estimate_csd | ||||
>>> from elephant.current_source_density_src.utility_functions import gauss_1d_dipole # noqa | ||||
>>> # 1. Define an array xs to x coordinate values with a length of 2304 | ||||
>>> xs=np.linspace(0, 10, 2304) | ||||
|
||||
>>> # 2. Run generate_lfp(gauss_1d_dipole, xs) | ||||
>>> lfp = generate_lfp(gauss_1d_dipole, xs) | ||||
|
||||
>>> # 3. Run estimate_csd(lfp, method="StandardCSD") | ||||
>>> csd = estimate_csd(lfp, method="StandardCSD") #doctest: +ELLIPSIS | ||||
discrete ... | ||||
>>> # 4. Print the results | ||||
>>> print(f"LFPs: {lfp}") | ||||
LFPs: [[-0.01483716 -0.01483396 -0.01483075 ... 0.01219233 0.0121911 | ||||
0.01218986]] mV | ||||
>>> print(f"CSD estimate: {csd}") #doctest: +ELLIPSIS | ||||
CSD estimate: [[-1.00025592e-04 -6.06684588e-05 ... | ||||
""" | ||||
|
||||
def integrate_1D(x0, csd_x, csd, h): | ||||
|
@@ -297,6 +318,10 @@ def integrate_3D(x, y, z, csd, xlin, ylin, zlin, X, Y, Z): | |||
sigma = 1.0 | ||||
h = 50. | ||||
if dim == 1: | ||||
# Handle one dimensional case, | ||||
# see https://github.com/NeuralEnsemble/elephant/issues/546 | ||||
if len(x_positions.shape) == 1: | ||||
x_positions = np.expand_dims(x_positions, axis=1) | ||||
chrg_x = x | ||||
csd = csd_profile(chrg_x) | ||||
pots = integrate_1D(x_positions, chrg_x, csd, h) | ||||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume this TODO should be addressed in another future PR and not here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the reminder, I should have made this TODO more explicit.
One might consider to further discuss the expected input shape in a future PR or Issue (?)
I've now (fe7f9ec) updated the docstring to explicitly specify the expected shape of the inputs as a 2D column vector with dimensions 'N x 1 array.' In the example mentioned in the issue, an error occurred because a flat array (1D) was passed as input.
For now, documenting the expected input is a reasonable solution ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, sounds good!