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 parallelization possible on the dimensions other than trials (e.g. over channels) #369

Open
nazgulfg92 opened this issue Oct 26, 2022 · 5 comments
Labels
Documentation Documentation has to be written for the issue/feature/bug More info needed More information is needed from the person who reported this. Tests Test need to be written for the issue/feature

Comments

@nazgulfg92
Copy link

Is your feature request related to a problem? Please elaborate.
Parallelization appears to only run over trials. If there's not trial structure in the data (e.g. spontaneous activity), there doesn't seem to be a way to tell Syncopy to use channels as the dimension to parallelize.

Do you have a solution in mind? Please elaborate
Would be nice to add the possibility to use the channel dimension (or any other dimension that is not trials) on the data for parallelization. Perhaps let things run in parallel, if no trials, over the first non-singleton dimension of the data structure hierarchy (e.g. if no trials, then channels, etc.).

Not really a problem per se, more like a missing feature? Please elaborate
Please provide a description of what kind of functionality you want to see in Syncopy. If possible/applicable, please consider writing a few lines of (pseudo-)code to illustrate your thoughts.

Thanks!

@nazgulfg92 nazgulfg92 added the Feature Request A concrete request (as detailed as possible) to either add or change functionality. label Oct 26, 2022
@dfsp-spirit dfsp-spirit added the Documentation Documentation has to be written for the issue/feature/bug label Oct 26, 2022
@tensionhead
Copy link
Contributor

Thanks for bringing the deficits of our documentation to our attention @nazgulfg92! There is actually a "meta-parameter" available for all meta-functions (spy.freqanalysis, spy.preprocessing, spy.resampledata, ...) called chan_per_worker which allows exactly what you asked for: parallelization over channels. So to use it, you would for example give

fft_spec = spy.freqanalysis(data, chan_per_worker=50)

and so if you have say 200 channels but only 1 trial, this would spawn 4 parallel processes.
There are of course methods, like bascially everything offered by spy.connectivityanalysis, where this is not trivially possible, as for those measures cross-channel entities have to be calculated. For these cases, manually cutting the initial data into smaller channel portions (via spy.selectdata(data, channel=[..]) might help.

Please let me know if chan_per_worker works for you as expected 🙂

PS: Parallelization over different axis does not seem to have a real use case by now, but I acknowledge that this could be a cool feature some time in the future.

@nazgulfg92
Copy link
Author

nazgulfg92 commented Oct 27, 2022

Thanks! I tried yesterday to give it a try, since I got some inside knowledge talking to @pantaray... but it couldn't figure out how the parameter actually worked. I kept getting some errors trying to use it with chan_per_worker = 1, assuming it'd get me one worker per channel (a little bit backwards from the parameter name, I admit haha).
What you say helps a lot making things clearer in my head. I'll try it out and let you know :)

tensionhead added a commit that referenced this issue Nov 3, 2022
- addresses #369

Changes to be committed:
	modified:   doc/source/user/concepts.rst
@tensionhead
Copy link
Contributor

Hey @nazgulfg92 , did you had any luck with the chan_per_worker parameter? Thx for letting us know!

@nazgulfg92
Copy link
Author

Hi @tensionhead, I tried but for some reason I don't remember it didn't work out for me. Might have been something with the implementation itself, or perhaps that it suddenly didn't fit my workflow (I managed to get ACME working on our Slurm from my computer). I'll try and take a look to see if there are some traces why chan_per_worker didn't make the cut. Will keep you updated :)

@tensionhead tensionhead added the More info needed More information is needed from the person who reported this. label Jan 2, 2023
@tensionhead tensionhead added Tests Test need to be written for the issue/feature and removed Feature Request A concrete request (as detailed as possible) to either add or change functionality. labels Feb 20, 2023
@tensionhead
Copy link
Contributor

In any case we should add a unit test to veryify this functionality for the supported computations!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Documentation has to be written for the issue/feature/bug More info needed More information is needed from the person who reported this. Tests Test need to be written for the issue/feature
Projects
None yet
Development

No branches or pull requests

3 participants