CINPLA / expipe-cinpla-legacy

All that was expipe and more
GNU General Public License v3.0
2 stars 2 forks source link

expipe crash/AttributeError #2

Closed espenhgn closed 7 years ago

espenhgn commented 7 years ago

Hi @lepmik, @alejoe91 and others. I've had a pretty frustrating day after updating to the latest commit d75ff89 (as the wiki documentation was no longer in sync with the version I had before).

First, after running python setup.py develop --extra browser I'm getting for any expipe execution the error:

$ expipe
Error when loading plugin `<class 'expipe_plugin_cinpla.main.CinplaPlugin'>`
Traceback (most recent call last):
  File "/Users/ehagen/COBRA/expipe-dev/expipe-cli/expipecli/utils/misc.py", line 56, in __getattribute__
    return object.__getattribute__(self, attr)
AttributeError: '_LazyImport' object has no attribute 'POSSIBLE_TAGS'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/ehagen/anaconda3/envs/expipe/bin/expipe", line 11, in <module>
    load_entry_point('expipe-cli', 'console_scripts', 'expipe')()
  File "/Users/ehagen/anaconda3/envs/expipe/lib/python3.6/site-packages/pkg_resources/__init__.py", line 570, in load_entry_point
    return get_distribution(dist).load_entry_point(group, name)
  File "/Users/ehagen/anaconda3/envs/expipe/lib/python3.6/site-packages/pkg_resources/__init__.py", line 2687, in load_entry_point
    return ep.load()
  File "/Users/ehagen/anaconda3/envs/expipe/lib/python3.6/site-packages/pkg_resources/__init__.py", line 2341, in load
    return self.resolve()
  File "/Users/ehagen/anaconda3/envs/expipe/lib/python3.6/site-packages/pkg_resources/__init__.py", line 2347, in resolve
    module = __import__(self.module_name, fromlist=['__name__'], level=0)
  File "/Users/ehagen/COBRA/expipe-dev/expipe-cli/expipecli/main.py", line 85, in <module>
    load_cli_plugins(expipe)
  File "/Users/ehagen/COBRA/expipe-dev/expipe-cli/expipecli/main.py", line 81, in load_cli_plugins
    raise e
  File "/Users/ehagen/COBRA/expipe-dev/expipe-cli/expipecli/main.py", line 78, in load_cli_plugins
    plugin().attach_to_cli(cli)
  File "/Users/ehagen/COBRA/expipe-dev/expipe-plugin-cinpla/expipe_plugin_cinpla/main.py", line 68, in attach_to_cli
    cli_misc.attach_to_cli(cli)
  File "/Users/ehagen/COBRA/expipe-dev/expipe-plugin-cinpla/expipe_plugin_cinpla/cli_misc.py", line 24, in attach_to_cli
    envvar=PAR.POSSIBLE_TAGS,
  File "/Users/ehagen/COBRA/expipe-dev/expipe-cli/expipecli/utils/misc.py", line 59, in __getattribute__
    self._esky_lazy_load()
  File "/Users/ehagen/COBRA/expipe-dev/expipe-cli/expipecli/utils/misc.py", line 45, in _esky_lazy_load
    self._esky_lazy_target = self._esky_lazy_loader()
  File "/Users/ehagen/COBRA/expipe-dev/expipe-plugin-cinpla/expipe_plugin_cinpla/imports.py", line 179, in PAR
    return load_parameters()
  File "/Users/ehagen/COBRA/expipe-dev/expipe-plugin-cinpla/expipe_plugin_cinpla/config.py", line 149, in load_parameters
    PAR = load_python_module(settings['current']['params'])
KeyError: 'params'

Second, you have to consider adapting to a workflow using issues, bugfix/feature branches, pull requests, and merges that also squash the commit history for individual issues. Right now mostly everything is done using a ton of incremental commits on branch:master and there is no way to tell what was the last good commit as there are no travisCI testing with this repository, and the revision history is a incomprehensible mess. There are reasons why commit messages should be descriptive ;-)

lepmik commented 7 years ago

Hi, the latest version in master is stable, this is a development repo that gathers all the tools and software that is used in the lab and has been through large updates the last couple of weeks. The respective sub-repos are more stable and should have a more clean history and is covered by travisCI testing. The wiki you are looking for is at expipe-plugin-cinpla wiki which also tells you how to set up your environment properly (which will fix your error).

I am truly sorry about your frustrating day, but this repo have been developed "on the fly" i.e. when something is not working properly we want to be able to fix and update immediately. Therefore we have omitted the standard workflow you mention until the usage is stable. Anyhow, there are no large updates to be expected soon so if there are no large issues in the near future we will update the sub-repos and tag those as stable.

lepmik commented 7 years ago

And I guess I also should mention that the reason for the "ton of incremental commits" is to keep commits confined to its respective subrepo - it should look more consistent when "subrepo pushed"

espenhgn commented 7 years ago

Hei! I did try and follow the stuff on the expipe-plugin-cinpla wiki earlier, which did not work because of some existing content in ~/.config/expipe/cinpla_plugin.yaml. It still appears half broken, as expipe env set my_project_id --params my_project_id.py (with some parameter file from Malin), will set the params value in the cinpla_plugin.yaml to null. That in turn throws an error with expipe. Manually setting the file path in the file appears to be working now though. Setting a probe path worked.

I'm heading to bed now, but I will have to look into the registration/spikesorting etc. tomorrow.

lepmik commented 7 years ago

Just delete the old file, and then use expipe env set ...

  1. sep. 2017 14.45 skrev "Espen Hagen" notifications@github.com:

Hei! I did try and follow the stuff on the expipe-plugin-cinpla wiki earlier, which did not work because of some existing content in ~/.config/expipe/cinpla_plugin.yaml. It still appears half broken, as expipe env set my_project_id --params my_project_id.py (with some parameter file from Malin), will set the params value in the cinpla_plugin.yaml to null. That in turn throws an error with expipe. Manually setting the file path in the file appears to be working now though. Setting a probe path worked.

I'm heading to bed now, but I will have to look into the registration/spikesorting etc. tomorrow.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/CINPLA/expipe-dev/issues/2#issuecomment-328667899, or mute the thread https://github.com/notifications/unsubscribe-auth/ADQn8LoPuAenO7-ZI_teknS-TumbyM4wks5shanqgaJpZM4PTYry .

espenhgn commented 7 years ago

Ok @lepmik, @alejoe91, @malinroe, I now had some success utilizing expipe* (hurra!), but had to modify certain parts in expipe_plugin_cinpla (https://github.com/CINPLA/expipe-dev/compare/master...espenhgn:misc_fixes?expand=1). But, I have some remaining questions/comments:

My workflow below. Other than this, anything I should do differently?

source activate expipe
expipe env set espen_test --params /Users/ehagen/COBRA/Ephys_data/data/espen_test.py --probe /Users/ehagen/COBRA/Ephys_data/data/espen_test/tetrodes32ch-klusta-oe.prb

# minimal registration
expipe register-subject 1871 --location=room2 --birthday 20.07.2017 --overwrite
expipe register-surgery 1871 --date now --procedure implantation --weight 400 g --position "v1l 0 0 0 0 mm" --angle "v1l 0 deg" --overwrite
expipe adjust 1871 --init #add depth info

source activate phy
# Register/copy raw data
expipe openephys register espen_test/acquisition_1871-200717-03/1871_2017-07-20_15-19-22_3/ --overwrite --no-move --no-module  

# modify probe file before
expipe openephys process 1871-200717-03 --filter-low 500 --filter-high 6000 --common-ref cmr --nchan 32 --ground 5 --ground 31 --no-tracking

# manual spike sorting
phy neo-gui /Users/ehagen/expipe_temp_storage/espen_test/1871-200717-03/main.exdir --channel-group 0
phy neo-gui /Users/ehagen/expipe_temp_storage/espen_test/1871-200717-03/main.exdir --channel-group 1

# post-processomg
expipe generate-notebook 1871-200717-03 --run
expipe analyse 1871-200717-03 --channel-group 0 -a spike-stat -a psd -a spike-lfp
expipe annotate 1871-200717-03 --tag test --message "just fucking around"

# upload
expipe transfer 1871-200717-03 --from-local

File espen_test.py (based on malin_cobra_probe.py written by either you or @alejoe91):

import quantities as pq

'''
This is an example parameter file to use with expipe-plugin-cinpla.

If you add _inherit_ before the name of a template it is asumed that you
want to "inherit" this module from the project. This is for modules
that are general and which you dont want to repeat in every action.
Note that you can modify these modules afterwords, then the modification
will be overloaded for that particular action.

Reserved names:
    subject
    electrophysiology
    implantation
    injection
'''

#  these are templates you want to add to each recording perfomed with
#  open ephys
openephys_templates = [
    '_inherit_hardware_openephys_daq',
    '_inherit_hardware_intan_headstage',
    '_inherit_hardware_pointgrey_camera_objective',
    '_inherit_software_bonsai_gui',
    '_inherit_software_openephys_gui',
    #'_inherit_environment_open_field_tracking',
    '_inherit_environment_rat_housing',
]

#  these are templates you want to add to each optogenetics session perfomed
#  with openephys
opto_openephys_templates = [
    '_inherit_hardware_openephys_optogenetics',
    '_inherit_hardware_pulse_pal',
    '_inherit_hardware_blue_laser',
    '_inherit_hardware_laser_measure_device',
    'malin_laser_settings',
    'malin_pulse_pal_settings',
    'malin_optogenetics_paradigm',
    'malin_optogenetics_anatomical_location'
]

#  thse are e.g. the brain areas you investigate, this ensures that you
#  register with with the same nomenclature for every recording
POSSIBLE_BRAIN_AREAS = ['V1BL', 'V1BR', 'V1ML', 'V1MR']
POSSIBLE_OPTO_TAGS = ['opto-train']
POSSIBLE_LOCATIONS = ['room2', 'room1']

#  obligatory tags will be enforced uppon transfer, this is to ensure you
#  have a minimum of tags for each action
OBLIGATORY_TAGS = ['no', 'yes', 'maybe']
POSSIBLE_TAGS = ['OS', 'MROS', 'baseline', 'test'] + OBLIGATORY_TAGS

#  thse are the templates you want to load to each suregery implantation
#  procedure
surgery_implantation_templates = [
    'malin_anaesthesia',
    'malin_analgesia',
    'malin_analgesia_post',
    'malin_anaesthesia_local',
    #'_inherit_malin_optic_fibre',
    '_inherit_malin_tetrode',
    #'_inherit_malin_drive_optetrode',
    # '_inherit_environment_mouse_housing',
    '_inherit_environment_surgery_station'
]

# thse are the templates you want to load to each suregery injection procedure
surgery_injection_templates = [
    'malin_anaesthesia',
    'malin_analgesia',
    'malin_analgesia_post',
    'malin_anaesthesia_local',
    # '_inherit_environment_mouse_housing',
    '_inherit_environment_surgery_station'
]

perfusion_templates = [
    'malin_perfusion_procedure',
    '_inherit_environment_surgery_station'
]

# this is how you represent each unit when you use expipe register-units
UNIT_INFO = {
    'info_waveform': {
        'alternatives': {
            'BS': 'definition: broad spiking waveform (putative excitatory)',
            'NS': 'definition: narrow spiking waveform (putative inhibitory)'
        },
        'value': ""
    },
    'info_pheno_type': {
        'value': ""
    }
}

# these are the analysis parameters you pass to exana
ANALYSIS_PARAMS = {
    'speed_filter': 5 * pq.m / pq.s,
    'pos_fs': 100 * pq.Hz,
    'f_cut': 6 * pq.Hz,
    'spat_binsize': 0.02 * pq.m,
    'spat_smoothing': 0.025,
    'grid_stepsize': 0.1 * pq.m,
    'box_xlen': 1 * pq.m,
    'box_ylen': 1 * pq.m,
    'ang_binsize': 4,
    'ang_n_avg_bin': 4,
    'imgformat': '.png',
    'corr_bin_width': 0.01 * pq.s,
    'corr_limit': 1. * pq.s,
    'isi_binsize': 1 * pq.ms,
    'isi_time_limit': 100 * pq.ms,
}

# this is personal user parameters
USER_PARAMS = {
    'project_id': 'espen_test',
    'user_name': 'Espen Hagen',
    'location': 'room2',
    'laser_device': {'name': 'hardware_blue_laser', 'id': 4}
}

# this is a placeholder for all your templates
TEMPLATES = {
    'openephys': openephys_templates,
    'opto_openephys': opto_openephys_templates,
    'surgery_implantation': surgery_implantation_templates,
    'surgery_injection': surgery_injection_templates,
    'perfusion': perfusion_templates,
    'adjustment': 'malin_drive_depth_adjustment'
}

MODULES = {
    'implantation': {'v1l': 'malin_implant_probe_vl',
                     'v1r': 'malin_implant_probe_vr',
                     'lgnl': 'malin_implant_fibre_lgnl',
                     'lgnr': 'malin_implant_fibre_lgnr'},
    'injection': {'lgnl': 'malin_injection_lgnl',
                  'lgnr': 'malin_injection_lgnr',
                  'v1l': 'malin_injection_v1l',
                  'v1r': 'malin_injection_v1r'},
    'subject': 'malin_subject_mouse'
}
alejoe91 commented 7 years ago

Hi @espenhgn, I can try to answer to a couple of questions, and maybe @lepmik can comment on the scp stuff:

Cheers Alessio

lepmik commented 7 years ago

@espenhgn

Ok @lepmik, @alejoe91, @malinroe, I now had some success utilizing expipe* (hurra!), but had to modify certain parts in expipe_plugin_cinpla (https://github.com/CINPLA/expipe-dev/compare/master...espenhgn:misc_fixes?expand=1). But, I have some remaining questions/comments:

Great! Thank you for the fix, strange that this has not been an issue before, but now it is fixed anyhow.

I could not get the expipe transfer ACTION-ID --to-local parts to run; presumably the data I wanted was set with wrong permissions, i.e., for the files .../malin_cobra/1871-200717-03/*, or didn't exist at all. See also last point.

Permissions must be altered post hoc or you can set it in $HOME/.profile found in each user on NIRD, speak with the norstore/NIRD support team for questions regarding permission.

The expipe openephys register ACTION-ID action defaults to copy/deleting the raw data. The --no-move option must be made default.

I disagree, as this will duplicate files and fill up the disk very quickly in the lab when you do many recordings. When doing the experiments you have many things to think about, and deleting the raw data after each time you issue the register command is prone to human error, i.e. deleting a file you thought you had registered but did not. Remember, if you set the data_path in .config/expipe/config.yaml to e.g. ~/expipe_temp_storage the register command will move files there.

For processing I added a --no-tracking option. The dataset I used may not have included tracking data. If it was, in what file type/name may I find it?

It makes sense to have this option. Maybe you can look at the code here to see how tracking data is read. NOTE: this entire block concerns tracking and should be put under the no-tracking option and be at the same indent level as no-convert

With regards to the spike sorting, I'm confused as to why phy neo-gui /Users/ehagen/expipe_temp_storage/espen_test/1871-200717-03/main.exdir --channel-group 0 results in different phy output than phy kwik-gui

They should show the same output, except the raw trace as you mention. The neo gui was mainly written to enable spikesorting of both data from axona and openephys. I do not think it wisely to use much time in including raw trace. Also there are other spike sorters available which might be more interesting to include in the workflow. Note that you can use --no-convert to omit outputting the clustering, (Note you must first fix the Note above) then spike sort with the kwik gui and running expipe openephys convert-klusta-oe. Note: if the feature view is not visible usually this is due to problems with the graphics card driver. Also when this happens, sometimes the waveforms view are affected even though it is shown.

My workflow below. Other than this, anything I should do differently?

The workflow seems fine, only you should set the location for subject registration to IBV as this location should point to which animal facility the animal is located at.

expipe register-subject 1871 --location=ibv --birthday 20.07.2017
espenhgn commented 7 years ago

Hey @lepmik and @alejoe91. Thanks.

lepmik commented 7 years ago

For the no_move option, let me try and explain why I think it is wrong that the default action is to delete files. It solves only the practical problem of having not-enough HD space. It does however defy common standards of not having the user at least confirm by input that certain files will be removed, it prevents the user to sensibly attempt to re-register the data if something goes horribly wrong downstream in the workflow (happened in my case because me==stupid), the suggested data_path = ~/expipe_temp_storage gives a connotation that files in there are just temporary working files but in fact contains everything on exdir format which again raises some philosophical questions whether or not this counts as raw data anymore (we should think about it), finally potential problems can arise if the dear expipe user also wants to run separate analysis on the raw data that is not integrated with expipe, but whoops where did the actual data go again? It is probably better to introduce a separate cleanup action that can be run after the data has been uploaded onto permanent storage facilities.

I see your point, a user input querying the deletion should be added, you can use this query_yes_no function.

As you can see, the channel IDs are all 6 on the right side, but with kwik-gui these values matches the channel IDs where the units were assigned (see also "Depth" and "Similarity").

Yes, I did not bother spending time on this when I wrote the NEOGUI, but it should be easy to fix, editing these lines

For your issue with the feature view I would go ask the phy forum, as this is most likely something with mac vs phy and not phy-contrib (given that the gui works on both linux and windows)

Do you have a suggestion to superimpose detected spikes onto raw signals other than checking with kwik-gui?

I would just code it in a notebook or similar, but if you rather want to play around with phy-contrib you are very welcome to add a raw signal to the neo-gui, then you will get some issues solved for free from the kwik gui, the code is quite similar.

Is the PCA scores saved in exdir format?

The PCA scores are stored in FeatureExtraction; see here for reference.

espenhgn commented 7 years ago

Hi. I now fixed this issue I had with showing cluster depth etc with the phy neo-gui option. The output is slightly different from the phy kwik-gui output but I guess this is expected as the neo-gui plugin appears to be based on the template-gui extension. Another question I have is; how can the memcache be flushed without commenting out functions in the memcache list in neo-gui?

espenhgn commented 7 years ago

To add: just pushed the change where the user has to confirm raw file deletion.

lepmik commented 7 years ago

Another question I have is; how can the memcache be flushed without commenting out functions in the memcache list in neo-gui?

You could delete the memcache, however there is probably some better way to do it. I think you would have to ask Cyrille on the phy forums to get the most complete answer.

Thank you for all your contributions

espenhgn commented 7 years ago

Ok. Thanks. Closing this.