Adding pipelines for cli analysis

Parvfect commented 2 years ago

As discussed, it makes sense to transition from the notebooks for analysis of eegnb experiments to a cli that can provide the user an opportunity to do the analysis without getting his hands messy with the code. This is what is being implemented in this PR.

As of now, automating the two experiments VisualN170 and VisualP300 analysis by callable functions whose use is specified in the code and repeated here,


#Visual N170:
raw, epochs = load_eeg_data('visual-n170', device_name='muse2016_bfn')
make_erp_plot(epochs)

#Visual P300:
raw, epochs = load_eeg_data('visual-P300', device_name='muse2016', event_id={'Non-Target': 1, 'Target': 2})
make_erp_plot(epochs, conditions=OrderedDict(NonTarget=[1],Target=[2]))

Some discussion to be had on implementing all experiments and where this can go, but a start to make sure that there is something to talk about.

Implemented as a part of the GSOC period for EEG - Notebooks under the mentorship of John Griffiths.

Parvfect commented 2 years ago

Follows the work done by @JohnGriffiths in https://github.com/NeuroTechX/eeg-notebooks/blob/enh_pipelines/eegnb/analysis/pipelines.py

hbk008 commented 2 years ago

The results are reproducible for N170 (in line with the tutorial for load and visualize data) but NOT for P300 due to differences in tmax value and reject={'eeg': } values along with differences in plotting values. I believe we need to make CLI plots in line with those in tutorials, right?

Parvfect commented 2 years ago

The results are reproducible for N170 (in line with the tutorial for load and visualize data) but NOT for P300 due to differences in tmax value and reject={'eeg': } values along with differences in plotting values. I believe we need to make CLI plots in line with those in tutorials, right?

yeah you're right, I'll have a look and fixing it

Parvfect commented 2 years ago

Current Status:

Automated Report generation done, looks like this

The Report gets saved in the appropiate save directory as per """ Analysis report saved to C:\Users\Parv/.eegnb\analysis\visual-N170\local\muse2\subject1\session3 """

Autoscaling for the plots is done using matplotlib's provided function (amen to that)

plt.autoscale(tight=True)

That fits it to the limits of the data which is what we want.

Some errors when trying automated generation after a live recorded session that need to be handled, would appreciate help if anyone has time with that.

PDF needs to be beautified with more explanation and made cleaner with the images aspect ratio.

For Reviewer

Kindly

Look through code structure in pipelines.py to make sure it is good programming practice. Currently for the report have to save two images so I can add it to the pdf and then delete them once its made. Don't know if that's really hacky or bad.
PDF gets automatically generated if analysis function is invoked, should there be an option to disable that?
Do we need an interface for the cli that prompts users for inputs or should I leave it as it is that can be used with two lines of code?
What should be added to the analysis report besdies some basic descriptive text, subject and session info?

JohnGriffiths commented 2 years ago

The overall structure looks good on first pass.

Some initial comments below.

Please address these and ensure the code is running and/or erroring at the expected location, and I will aim to give it a full run through.

Re: Python code -

You need to give the complete code required to run the new function in the PR documentation. The above did not include the relevant imports, so I had to go and find where these functions were implemented myself.
there should be a fetch_dataset call either within the function itself within the code, or somewhere in the surrounding code. (The P300 initially didn't work for me, because it hadn't been downloaded yet on this comp)
I could run successfully with the above python API lines once imported and data in the right place.
I think the matplotlib figures need to be closed automatically (perhaps after some timeout); as the function hangs until they are manually clicked shut
The 'file created' message should not only list the folder created in, but the actual file name. Currentlly it doesn't do this.

Re: CLI -

Please add CLI usage description to the PR info
I am also getting this error now at the CLI call.

(eeg-notebooks) C:\Users\eeg_lab\Code\github\eeg-notebooks_dev\eeg-noteoboks_pullrequestreviews>eegnb create-analysis-report -ex visual-n170
Usage: eegnb [OPTIONS]
Try 'eegnb --help' for help.

Error: Got unexpected extra arguments (v i s u a l - n 1 7 0)

(which you should also document in the PR. )

Re: the report:

It's nice but figures are still getting stretched and clipped off the screen
FYI, some example HTML analysis reports from the Kernel portal here, here, here. Ultimately we are aiming at something broadly like this. For now I'm thinking more the layout than the interactive graphs, but that would be a very nice touch also. FSL (neuroimaging software) has been providing QC and results files in kind of format for > 20 years.

hbk008 commented 2 years ago

I get this error message as I am testing with example data. Seems like example = True does not fetch the example data.

Parvfect commented 2 years ago

I get this error message as I am testing with example data. Seems like example = True does not fetch the example data.

It's cause you're passing in muse2 as the device, try removing device, subject, session, so it's like

raw,epochs = load_eeg_data('visual-N170', example=True)
make_erp_plot(epochs)

for the Visual N170.

I should have updated the PR though, so this should have been avoided, apologies.

Parvfect commented 2 years ago

Current State

Running cli through

eegnb create-analysis-report -ip

throws an error of extra argument, unsure why, trying to fix it, would appreciate some help. Changes made in eegnb/cli/main and eegnb/cli/intro_prompt and connects to create_analysis_report in eegnb/analysis/pipelines

main

intro_prompt

Running the analysis functions

Running example datasets

from eegnb.analysis.pipelines import example_analysis_report
example_analysis_report()

Alternatively,

from eegnb.analysis.pipelines import load_eeg_data, make_erp_plot
raw,epochs = load_eeg_data(experiment='visual-N170', example=True) # setting example true prompts everything
make_erp_plot(epochs)

Running analysis functions on local dataset

Ideally, trying to get cli to function, but for now,

from eegnb.analysis.pipelines import create_analysis_report
create_analysis_report(experiment, eegdevice, subject, session, datapath)

wherein passing subject and session is optional if passing datapath and vice versa

hbk008 commented 2 years ago

I get this error message as I am testing with example data. Seems like example = True does not fetch the example data.

It's cause you're passing in muse2 as the device, try removing device, subject, session, so it's like
raw,epochs = load_eeg_data('visual-N170', example=True)
make_erp_plot(epochs)
for the Visual N170.

I should have updated the PR though, so this should have been avoided, apologies.

Okay, this works now!

hbk008 commented 2 years ago

Just checked again, works for N170 but does not work for P300 with example = True. I can see that the device name is passed on line 256 for P300 when example = True. Can you check that again?

Parvfect commented 2 years ago

Just checked again, works for N170 but does not work for P300 with example = True. I can see that the device name is passed on line 256 for P300 when example = True. Can you check that again?

device changes again to muse_2016. I'd prefer you use the _example_analysisreport() function so it can handle the device issues

Parvfect commented 2 years ago

Things to complete on this (as discussed 1/9/22)

(TBC starting 20 September finishing and deploying by the end of the month)

Global Variables in pipelines.py to passed around dictionary parameters (to be completed in the Experiment Class Library as well)
Add an option of launching saved pdf using one line in the terminal for ease of use
PDF Beautification (section heading, hyperlinks, picture aspect ratio, information about experiment and linking to further explanation in documentation, including sample drop percentage and other relevant outputs)
Newline after terminal message for the inputs
Fix multiple images for the filters that get displayed
Add a ten second timer for automatically closing figures
Automatic analysis report for recorded session through cli
Add pdf library to requirements

@oreHGA, @retiutut, @ErikBjare, @JohnGriffiths - I would really appreciate a solid PR review with some feedback for this because I feel this is quite a cool feature that we can simply add to EEG Notebooks and can expand the user base as well as improve general functionality. Additionally, I do think that I have assumed a lot of things while writing this code because it was rushed, which might be ignored because everything works so could really appreciate some feedback.

General instructions for running it

eegnb create-analysis-report -ip

Specifically using pipeline functions

Usage: For Recorded Data:

from eegnb.analysis.pipelines import create_analysis_report()
create_analysis_report(experiment, eegdevice, subject, session, filepath)s

For Example Datasets:

from eegnb.analysis.pipelines import example_analysis_report()
example_analysis_report()

Parvfect commented 2 years ago

Things to complete on this (as discussed 1/9/22)

(TBC starting 20 September finishing and deploying by the end of the month)

Global Variables in pipelines.py to passed around dictionary parameters (to be completed in the Experiment Class Library as well)

Add an option of launching saved pdf using one line in the terminal for ease of use

PDF Beautification (section heading, hyperlinks, picture aspect ratio, information about experiment and linking to further explanation in documentation, including sample drop percentage and other relevant outputs)

Newline after terminal message for the inputs

Fix multiple images for the filters that get displayed

Add a ten second timer for automatically closing figures

Automatic analysis report for recorded session through cli

Add pdf library to requirements

@oreHGA, @retiutut, @ErikBjare, @JohnGriffiths - I would really appreciate a solid PR review with some feedback for this because I feel this is quite a cool feature that we can simply add to EEG Notebooks and can expand the user base as well as improve general functionality. Additionally, I do think that I have assumed a lot of things while writing this code because it was rushed, which might be ignored because everything works so could really appreciate some feedback.

General instructions for running it
eegnb create-analysis-report -ip
Specifically using pipeline functions

Usage: For Recorded Data:
from eegnb.analysis.pipelines import create_analysis_report()
create_analysis_report(experiment, eegdevice, subject, session, filepath)s
For Example Datasets:
from eegnb.analysis.pipelines import example_analysis_report()
example_analysis_report()

Things completed

Packaging global variables as experimental parameters
Added hyperlink to make it easy for user to see report from terminal
Added eegdevice list prompt for selection
Automatically close figures after 10 seconds of viewing them
Fixed requirements.txt
Embedded images into html using b64

Big change of switiching from pdf to generating html for stylistic purposes following comments left by John. Linking to different sections done, and image stretching etc has been resolved - here's what the template looks like

[ ]

The text is rudimentary and the taskbar links to different sections for easy navigation.

Testing

The way of testing this remains the same as before and is the previous comment in this thread that this one replies to but in simple terms,

eegnb create-analysis-report -ip

Things left to do

[x] Two images show up in fresh installation
[x] Link not shown in some terminals
[x] Automated report generation - filename passing test otherwise functional -> tabled
[x] Actual styling of report needs to be considered

Would really appreciate some quick feedback so we can get the ball rolling and get this done soon.

retiutut commented 2 years ago

Why are the checks still failing? It looks like all but one PR fails the tests 🤷‍♂️

Parvfect commented 2 years ago

Why are the checks still failing? It looks like all but one PR fails the tests 🤷‍♂️

I'm not completely sure but my thinking is that these things could be responsible,

Changed requirements by adding Airium
Changed the cli by adding automated analysis option

JohnGriffiths commented 2 years ago

It's the requirements change. If you look at the task list the red cross appears at install dependencies.

Parvfect commented 2 years ago

@ErikBjare I'm trying to figure out why there is a docs build error, shown as follows,

I see that you have updated something in the main branch about wxpython's wheels to PyPi with reference to #190

Although it seems to say that the module attrdict is not found, do you know what is actually happening because I am pretty confused. There aren't major changes in the cli that have to do with building wxpython so it's hard to understand why it's creating an issue. Would love some assistance.

JohnGriffiths commented 1 year ago

Finished with basic functionality testing. Merging now.

I have some improvements to the report generation functionality and some of the function options, will add those as a new PR rather than over-complicating this one.

( Note, the doc build issue here is unrelated to the content of this PR, seems due to a wxpython issue that came up whilst this PR was made. So will address that separately and not on this PR. )

Good job @Parvfect !

NeuroTechX / EEG-ExPy