EEG-ExPy icon indicating copy to clipboard operation
EEG-ExPy copied to clipboard

Adding pipelines for cli analysis

Open Parvfect opened this issue 3 years ago • 3 comments

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.

Parvfect avatar Aug 08 '22 16:08 Parvfect

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

Parvfect avatar Aug 08 '22 16:08 Parvfect

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?

hbk008 avatar Aug 17 '22 15:08 hbk008

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 avatar Aug 17 '22 16:08 Parvfect

Current Status:

Automated Report generation done, looks like this image

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

  1. 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.
  2. PDF gets automatically generated if analysis function is invoked, should there be an option to disable that?
  3. 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?
  4. What should be added to the analysis report besdies some basic descriptive text, subject and session info?

Parvfect avatar Aug 28 '22 20:08 Parvfect

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.

JohnGriffiths avatar Aug 31 '22 17:08 JohnGriffiths

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

image

hbk008 avatar Aug 31 '22 22:08 hbk008

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

image

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 avatar Sep 01 '22 10:09 Parvfect

Current State

  1. 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

image

main

image

intro_prompt

image

Running the analysis functions

  1. 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

Parvfect avatar Sep 01 '22 11:09 Parvfect

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

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 avatar Sep 01 '22 16:09 hbk008

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?

hbk008 avatar Sep 01 '22 18:09 hbk008

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_analysis_report() function so it can handle the device issues

Parvfect avatar Sep 01 '22 19:09 Parvfect

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

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

  1. Global Variables in pipelines.py to passed around dictionary parameters (to be completed in the Experiment Class Library as well)
  2. Add an option of launching saved pdf using one line in the terminal for ease of use
  3. 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)
  4. Newline after terminal message for the inputs
  5. Fix multiple images for the filters that get displayed
  6. Add a ten second timer for automatically closing figures
  7. Automatic analysis report for recorded session through cli
  8. 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 avatar Sep 05 '22 01:09 Parvfect

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

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

  1. Global Variables in pipelines.py to passed around dictionary parameters (to be completed in the Experiment Class Library as well)
  2. Add an option of launching saved pdf using one line in the terminal for ease of use
  3. 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)
  4. Newline after terminal message for the inputs
  5. Fix multiple images for the filters that get displayed
  6. Add a ten second timer for automatically closing figures
  7. Automatic analysis report for recorded session through cli
  8. 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

  1. Packaging global variables as experimental parameters
  2. Added hyperlink to make it easy for user to see report from terminal
  3. Added eegdevice list prompt for selection
  4. Automatically close figures after 10 seconds of viewing them
  5. Fixed requirements.txt
  6. 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

image

  • [ ] image

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.

Parvfect avatar Sep 26 '22 11:09 Parvfect

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

retiutut avatar Sep 27 '22 13:09 retiutut

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,

  1. Changed requirements by adding Airium
  2. Changed the cli by adding automated analysis option

Parvfect avatar Sep 28 '22 10:09 Parvfect

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

JohnGriffiths avatar Sep 28 '22 11:09 JohnGriffiths

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

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

image

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.

Parvfect avatar Oct 06 '22 11:10 Parvfect

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 !

JohnGriffiths avatar Oct 16 '22 19:10 JohnGriffiths