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

Feature/multitaper psd estimate #417

Conversation

rjurkus
Copy link
Collaborator

@rjurkus rjurkus commented Apr 16, 2021

This PR covers the basic implementation of multitaper in Elephant. Tests include a comparison against nitime multitaper. Any feedback is appreciated!

Please note: The test against nitime will fail due to the missing .npy files. Please check if this style of the test is sufficient. If so, the files can be uploaded to the correct location and the test should then pass.

@mdenker mdenker added the enhancement Editing an existing module, improving something label May 17, 2021
@mdenker mdenker self-assigned this May 17, 2021
Comment on lines 279 to 280
If None, it will be determined from other parameters.
Default: None.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Where happens the determination of the peak_resolution from other parameters, if it was 'None'?

Copy link
Contributor

@ackurth ackurth Jun 14, 2021

Choose a reason for hiding this comment

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

If the peak_resolution is None, its value does not get derived from the other parameters since it is not needed for the subsequent steps. Only if peak_resolution is assigned an appropriate value, it is used to determine the time bandwidth product which in turn is used to computed num_tapers, which controls the peak resolution of the estimate.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Then If None, it will be determined from other parameters. is quite misleading since it's ambiguous what 'it' refers to

@mdenker mdenker added this to the v0.11.0 milestone May 21, 2021
rjurkus and others added 19 commits October 1, 2021 14:56
sym='False' -> sym=False (boolean parameter)
nperseg -> n_per_seg (consistency)
…ad data from the elephant-data repo (using test_spike_train_synchrony.py and test_unitary_event_analysis.py approach)

Replaced a large part of test_multitaper_psd_against_nitime docstring with a URL to the elephant-data repo, updated the mentioned nitime version and corrected a typo
@coveralls
Copy link
Collaborator

coveralls commented Dec 10, 2021

Coverage Status

Coverage decreased (-0.07%) to 88.615% when pulling d0a8601 on INM-6:feature/multitaper_psd_estimate into b637b2c on NeuralEnsemble:master.

Copy link
Member

@Moritz-Alexander-Kern Moritz-Alexander-Kern left a comment

Choose a reason for hiding this comment

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

Nice feature, thank you for contributing.

elephant/spectral.py Outdated Show resolved Hide resolved
elephant/test/test_spectral.py Outdated Show resolved Hide resolved
elephant/spectral.py Outdated Show resolved Hide resolved
elephant/spectral.py Outdated Show resolved Hide resolved
@Moritz-Alexander-Kern Moritz-Alexander-Kern merged commit d0d3920 into NeuralEnsemble:master Feb 10, 2022
Moritz-Alexander-Kern added a commit that referenced this pull request Feb 23, 2022
mdenker pushed a commit that referenced this pull request Feb 23, 2022
This reverts commit d0d3920 to fix an issue where the authors of #417 where incorrectly identified. #417 will be re-merged with correct author attributions.
@Moritz-Alexander-Kern Moritz-Alexander-Kern deleted the feature/multitaper_psd_estimate branch October 28, 2022 07:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new functionality New modules, functions
Projects
None yet
Development

Successfully merging this pull request may close these issues.