NeuroTechX / EEG-ExPy

EEG Experiments in Python
https://neurotechx.github.io/EEG-ExPy/
BSD 3-Clause "New" or "Revised" License
420 stars 124 forks source link

Experiment Class Refactor (update to #183), converting specific experiments to subclasses #184

Closed Parvfect closed 1 year ago

Parvfect commented 2 years ago

As per the comments left in PR #183, Sub classes make more sense for increasing modular support and as a general framework. I've implemented the base Experiment Class and the Experiment Subclasses for Visual P300, N170 and SSVEP (although I can rewrite some of the code in SSVEP and make it better). Next step is running it and making sure it works the same, I'm having some trouble in downloading the psychopy package due to my Python version and I should get that done very soon. Wanted to put this through so we could talk about it in the dev meeting.

Changes as per the project proposal for Google Summer of Code for eeg-notebooks - https://summerofcode.withgoogle.com/programs/2022/projects/2baQ46dm.

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

Parvfect commented 2 years ago

Right so the Visual P300 and the Visual N170 works although its a bit different,

''' _from eegnb.experiments.visualn170 import n170 t = n170.VisualN170() t.present() ''''

That should run it.

A few things worth pointing out,

  1. I have changed how the windows work, previously two mywin instances were used to show instructions and present whereas I have reused the first one - don't know if this is ideal and need some advice.
  2. I have a shit ton of self parameters to share variables across functions and I don't know if that's fun for someone new who tries to create an experiment from this structure -> might need to be discussed
  3. There are some bits of ugly code here and there that I am going to clean in the next few days
  4. Some inputs in some of the files seem redundant if it's importing the Experiment Class
Parvfect commented 2 years ago

Have adressed the comments given above and made the naming changes. Two issues still remaining

  1. For importing experiments directly the files will have to be removed from their folders that is from (VisualN170/n170.py) to just the file. Don't think this should cause any complications but need to make sure this is what is needed .
  2. I can't seem to get rid of the redundant imports in the specific experiments by putting in the BaseExperiment class, if anyone can help with that it would be lovely.

Besides that I think I should be good to go and start changing the other Experiments for this style, just need a nod from John to start it.

JohnGriffiths commented 2 years ago

Getting error with this.

Running

eegnb runexp -ip muse2_bfn

Gives

Traceback (most recent call last):
  File "C:\Users\eeg_lab\.conda\envs\eeg-notebooks\Scripts\eegnb-script.py", line 33, in <module>
    sys.exit(load_entry_point('eeg-notebooks', 'console_scripts', 'eegnb')())
  File "C:\Users\eeg_lab\.conda\envs\eeg-notebooks\lib\site-packages\click\core.py", line 1128, in __call__
    return self.main(*args, **kwargs)
  File "C:\Users\eeg_lab\.conda\envs\eeg-notebooks\lib\site-packages\click\core.py", line 1053, in main
    rv = self.invoke(ctx)
  File "C:\Users\eeg_lab\.conda\envs\eeg-notebooks\lib\site-packages\click\core.py", line 1659, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "C:\Users\eeg_lab\.conda\envs\eeg-notebooks\lib\site-packages\click\core.py", line 1395, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "C:\Users\eeg_lab\.conda\envs\eeg-notebooks\lib\site-packages\click\core.py", line 754, in invoke
    return __callback(*args, **kwargs)
  File "c:\users\eeg_lab\code\github\eeg-notebooks_dev\eeg-notebooks_reviewpr184\eegnb\cli\__main__.py", line 80, in runexp
    run_experiment(experiment, eeg, recdur, outfname)
  File "c:\users\eeg_lab\code\github\eeg-notebooks_dev\eeg-notebooks_reviewpr184\eegnb\cli\utils.py", line 45, in run_experiment
    module.present(duration=record_duration, eeg=eeg_device, save_fn=save_fn)  # type: ignore
AttributeError: module 'eegnb.experiments.visual_n170.n170' has no attribute 'present'
Parvfect commented 1 year ago

Getting error with this.

Running

eegnb runexp -ip muse2_bfn

Gives

Traceback (most recent call last):
  File "C:\Users\eeg_lab\.conda\envs\eeg-notebooks\Scripts\eegnb-script.py", line 33, in <module>
    sys.exit(load_entry_point('eeg-notebooks', 'console_scripts', 'eegnb')())
  File "C:\Users\eeg_lab\.conda\envs\eeg-notebooks\lib\site-packages\click\core.py", line 1128, in __call__
    return self.main(*args, **kwargs)
  File "C:\Users\eeg_lab\.conda\envs\eeg-notebooks\lib\site-packages\click\core.py", line 1053, in main
    rv = self.invoke(ctx)
  File "C:\Users\eeg_lab\.conda\envs\eeg-notebooks\lib\site-packages\click\core.py", line 1659, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "C:\Users\eeg_lab\.conda\envs\eeg-notebooks\lib\site-packages\click\core.py", line 1395, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "C:\Users\eeg_lab\.conda\envs\eeg-notebooks\lib\site-packages\click\core.py", line 754, in invoke
    return __callback(*args, **kwargs)
  File "c:\users\eeg_lab\code\github\eeg-notebooks_dev\eeg-notebooks_reviewpr184\eegnb\cli\__main__.py", line 80, in runexp
    run_experiment(experiment, eeg, recdur, outfname)
  File "c:\users\eeg_lab\code\github\eeg-notebooks_dev\eeg-notebooks_reviewpr184\eegnb\cli\utils.py", line 45, in run_experiment
    module.present(duration=record_duration, eeg=eeg_device, save_fn=save_fn)  # type: ignore
AttributeError: module 'eegnb.experiments.visual_n170.n170' has no attribute 'present'

So this is because we changed our present function to run. Additionally, the runexp script will need to be modified because the usage is not eegnb.experiments.visual_n170.n170 anymore and we have to initalize a class object first of the form,

'from eegnb.devices.eeg import EEG from eegnb.experiments import VisualN170, VisualP300, VisualSSVEP, AuditoryOddball

if name == "main": board_name = 'muse2'

record_duration = 120
eeg_device = EEG(device=board_name)
t = AuditoryOddball(duration=record_duration, eeg=eeg_device)
t.run() 

'

I'm not too sure where the runexp file is, I'll try finiding it right now so I can make the necessary changes.

Parvfect commented 1 year ago

Okay so I tested each experiment changed (VisualN170, VisualP300, VisualSSVEP and AuditoryOddball) using the Muse2 and a script importing them directly rather than through runexp. They all seem to be functional, no errors and running smooth. There seems to be a slight delay from the spacebar of the instruction screen to the displaying of images, which I assume is setting up the eeg-stream, but it's definitely longer than normal and I'll need another pair of eyes on it to see if it's a problem.

As of now, I need to fix the cli usage of it as John has shown above and more robust tests for making sure everything is working as we need. I also have not been able to test this through the Brainflow, have been using Bluemuse.

Parvfect commented 1 year ago

Getting error with this. Running

eegnb runexp -ip muse2_bfn

Gives

Traceback (most recent call last):
  File "C:\Users\eeg_lab\.conda\envs\eeg-notebooks\Scripts\eegnb-script.py", line 33, in <module>
    sys.exit(load_entry_point('eeg-notebooks', 'console_scripts', 'eegnb')())
  File "C:\Users\eeg_lab\.conda\envs\eeg-notebooks\lib\site-packages\click\core.py", line 1128, in __call__
    return self.main(*args, **kwargs)
  File "C:\Users\eeg_lab\.conda\envs\eeg-notebooks\lib\site-packages\click\core.py", line 1053, in main
    rv = self.invoke(ctx)
  File "C:\Users\eeg_lab\.conda\envs\eeg-notebooks\lib\site-packages\click\core.py", line 1659, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "C:\Users\eeg_lab\.conda\envs\eeg-notebooks\lib\site-packages\click\core.py", line 1395, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "C:\Users\eeg_lab\.conda\envs\eeg-notebooks\lib\site-packages\click\core.py", line 754, in invoke
    return __callback(*args, **kwargs)
  File "c:\users\eeg_lab\code\github\eeg-notebooks_dev\eeg-notebooks_reviewpr184\eegnb\cli\__main__.py", line 80, in runexp
    run_experiment(experiment, eeg, recdur, outfname)
  File "c:\users\eeg_lab\code\github\eeg-notebooks_dev\eeg-notebooks_reviewpr184\eegnb\cli\utils.py", line 45, in run_experiment
    module.present(duration=record_duration, eeg=eeg_device, save_fn=save_fn)  # type: ignore
AttributeError: module 'eegnb.experiments.visual_n170.n170' has no attribute 'present'

So this is because we changed our present function to run. Additionally, the runexp script will need to be modified because the usage is not eegnb.experiments.visual_n170.n170 anymore and we have to initalize a class object first of the form,

'from eegnb.devices.eeg import EEG from eegnb.experiments import VisualN170, VisualP300, VisualSSVEP, AuditoryOddball

if name == "main": board_name = 'muse2'

record_duration = 120
eeg_device = EEG(device=board_name)
t = AuditoryOddball(duration=record_duration, eeg=eeg_device)
t.run() 

'

I'm not too sure where the runexp file is, I'll try finiding it right now so I can make the necessary changes.

Fixed this in new commit. Please note that an if condition had to be added for the new calling style that can be made cleaner if all the experiments are adapted.

Code looks like this (eegnb/cli/utils.py) for initialization

from eegnb.experiments import VisualN170
from eegnb.experiments import VisualP300
from eegnb.experiments import VisualSSVEP
from eegnb.experiments import AuditoryOddball
from eegnb.experiments.visual_cueing import cueing
from eegnb.experiments.visual_codeprose import codeprose
from eegnb.experiments.auditory_oddball import diaconescu
from eegnb.experiments.auditory_ssaep import ssaep, ssaep_onefreq

# New Experiment Class structure has a different initilization, to be noted
experiments = {
    "visual-N170": VisualN170(),
    "visual-P300": VisualP300(),
    "visual-SSVEP": VisualSSVEP(),
    "visual-cue": cueing,
    "visual-codeprose": codeprose,
    "auditory-SSAEP orig": ssaep,
    "auditory-SSAEP onefreq": ssaep_onefreq,
    "auditory-oddball orig": AuditoryOddball(),
    "auditory-oddball diaconescu": diaconescu,
}

and for the running,

def run_experiment(
    experiment: str, eeg_device: EEG, record_duration: float = None, save_fn=None
):
    if experiment in experiments:
        module = experiments[experiment]

        # Condition added for different run types of old and new experiment class structure
        if experiment == "visual-N170" or experiment == "visual-P300" or experiment == "visual-SSVEP" or experiment == "auditory-oddball orig":
            module.duration = record_duration
            module.eeg = eeg_device
            module.save_fn = save_fn
            module.run()
        else:
            module.present(duration=record_duration, eeg=eeg_device, save_fn=save_fn)  # type: ignore
    else:
        print("\nError: Unknown experiment '{}'".format(experiment))
        print("\nExperiment can be one of:")
        print("\n".join([" - " + exp for exp in experiments]))
Parvfect commented 1 year ago

For Reviewer

Test each experiment (Visual N170, Visual P300, Auditory Oddball, Visual SSVEP) using the CLI using the EEG (already working without)

eegnb runexp -ip

There seems to be a slight delay between the instruction screen and running the experiments that is bothering me, really need someone to confirm that it's alright. Besides that everything else seems alright.

JohnGriffiths commented 1 year ago

This isn't working for me. I get 1 image in the sequence shown, then it closes down immediately. Tried for Muse 2016 and Muse S on Windows 10 ( I think). Trying to get to the bottom of it.

Parvfect commented 1 year ago

This isn't working for me. I get 1 image in the sequence shown, then it closes down immediately. Tried for Muse 2016 and Muse S on Windows 10 ( I think). Trying to get to the bottom of it.

Send me any error messages or outputs if you can, could look into it and find out more.

JohnGriffiths commented 1 year ago

Ok, apols for false alarm. This appears to be working ok for me now. Tested for museS and muse2016 on two windows 10 comps, with brainflow native device option.

I agree that the wait period between pressing space and the experiment starting seems a bit long. I wonder if that can be sped up?

If that is difficult then the message could be adjusted to something like

'please press space and experiment will begin in a few seconds'

Haven't yet checked that the ERPs recorded with this look ok. No real reason why they shouldn't but is obvs super important that that is confirmed. Will get to that asap.

Will do a final review of code changes shortly, and if all is good then I am good to merge this PR.

JohnGriffiths commented 1 year ago

Merging to dev.