PSTH #339

tensionhead · 2022-08-23T15:52:09Z

PSTH 🚀

added peri stimulus time histogram with a frontend API like in FT
one minimal functional test was included, general proper tests still missing
additional datasets like avg or var will get added once TimeLockData is up to speed

addresses #295

Author Guidelines

Was the code checked for memory leaks/performance bottlenecks?
Is the code running locally and on the ESI cluster?
Is the code running on all supported platforms?

Reviewer Checklist

Dev merge

Changes to be committed: modified: syncopy/datatype/__init__.py deleted: syncopy/datatype/statistical_data.py

Update from Dev

- got a working frontend call Changes to be committed: modified: syncopy/datatype/continuous_data.py modified: syncopy/spikes/compRoutines.py modified: syncopy/spikes/psth.py modified: syncopy/spikes/spike_psth.py

- fixed some remaining indexing issues - correct channel labels now Changes to be committed: modified: syncopy/datatype/continuous_data.py modified: syncopy/spikes/compRoutines.py modified: syncopy/spikes/psth.py modified: syncopy/spikes/spike_psth.py

- now it's time to make sense out of the histograms Changes to be committed: modified: syncopy/spikes/compRoutines.py modified: syncopy/spikes/spike_psth.py

- totally random trial definition can produce trials with no events, which causes downstream problems for indexing - massaged the synth generator to always have at least 90% of average trial size nSamples .// nTrials - this might be a problem in RL as well if ppl define trials in empty regions.. Changes to be committed: modified: syncopy/tests/synth_data.py

- added NaN masking for time bins if trial outside of latency - TODO: fix time axis / trialdefinition Changes to be committed: modified: syncopy/datatype/continuous_data.py modified: syncopy/datatype/discrete_data.py modified: syncopy/spikes/compRoutines.py modified: syncopy/spikes/psth.py modified: syncopy/spikes/spike_psth.py modified: syncopy/tests/synth_data.py

- should be investigated why the manual construction of the Indexer to access selected trials outside the CR was necessary Changes to be committed: modified: syncopy/spikes/compRoutines.py modified: syncopy/spikes/psth.py modified: syncopy/spikes/spike_psth.py

Changes to be committed: modified: syncopy/spikes/psth.py new file: syncopy/tests/test_spike_psth.py

codecov · 2022-08-23T16:03:21Z

Codecov Report

Merging #339 (2122a46) into dev (5df8804) will increase coverage by 3.04%.
The diff coverage is 60.58%.

@@            Coverage Diff             @@
##              dev     #339      +/-   ##
==========================================
+ Coverage   65.84%   68.89%   +3.04%     
==========================================
  Files          77       77              
  Lines        8742     8850     +108     
  Branches     1784     1816      +32     
==========================================
+ Hits         5756     6097     +341     
+ Misses       2591     2335     -256     
- Partials      395      418      +23

Impacted Files	Coverage Δ
syncopy/datatype/continuous_data.py	`83.37% <33.33%> (+1.36%)`	⬆️
syncopy/spikes/spike_psth.py	`43.36% <47.36%> (+20.78%)`	⬆️
syncopy/spikes/psth.py	`74.07% <76.00%> (+54.62%)`	⬆️
syncopy/spikes/compRoutines.py	`89.47% <80.95%> (+89.47%)`	⬆️
syncopy/datatype/__init__.py	`100.00% <100.00%> (ø)`
syncopy/datatype/base_data.py	`81.96% <100.00%> (-0.19%)`	⬇️
syncopy/datatype/discrete_data.py	`83.70% <100.00%> (+0.37%)`	⬆️
syncopy/tests/synth_data.py	`83.33% <100.00%> (+19.69%)`	⬆️
syncopy/nwanalysis/connectivity_analysis.py	`73.45% <0.00%> (-2.66%)`	⬇️
syncopy/shared/tools.py	`79.76% <0.00%> (-2.39%)`	⬇️
... and 20 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Changes to be committed: modified: syncopy/tests/synth_data.py

dfsp-spirit · 2022-08-24T14:17:04Z

syncopy/spikes/spike_psth.py

+        sinfo = data.sampleinfo
+        trials = data.trials
+
+    lenTrials = np.diff(sinfo).squeeze()


lenTrials this seems to be unused

True, this I will remove!

syncopy/spikes/spike_psth.py

dfsp-spirit · 2022-08-24T14:18:50Z

syncopy/spikes/spike_psth.py

    chan_labels = [f'channel{i}_unit{j}' for i, j in combs]

    # now we have our global (single-trial, avg, std,..) histogram shape
    h_shape = (nBins, len(combs))


this is not part of this change set, but chan_labels and h_shape are also unused

Also here, it's for book keeping of that complicated output shape, yet the chan_labels already get used in process_metadata, so I'll remove 'em.

dfsp-spirit · 2022-08-24T14:24:13Z

The docstring for the frontend, spike_psth(), lists a boolean parameter vartriallen that does not exist. Do we want to implement this later? Otherwise it should be removed.

dfsp-spirit · 2022-08-24T14:26:31Z

The output parameter for the frontend, spike_psth(), seems to be ignored.

dfsp-spirit · 2022-08-24T14:36:01Z

syncopy/spikes/psth.py

    # now map with respect to unit for all single trial channels (-bins)
    map_unit = {u: np.logical_or.reduce([map_cu(c, u) for c in chan_vec]) for u in unique_units}

+    # map into histogram time x channel dimensions
+    map_unit_hist = {u: [np.any(map_cu(c, u)) for c in chan_vec] for u in unique_units}
+
    for i, iunit in enumerate(unique_units):


again not part of this changeset, but i is unused, so no need for enumerate

dfsp-spirit · 2022-08-24T14:42:07Z

syncopy/spikes/spike_psth.py

-from syncopy.datatype import SpikeData
+from syncopy.shared.tools import get_defaults, get_frontend_cfg
+from syncopy.datatype import TimeLockData
+from syncopy.datatype.base_data import Indexer

 from syncopy.shared.errors import SPYValueError, SPYTypeError, SPYWarning, SPYInfo


minor: SPYTypeError, SPYWarning, SPYInfo are imported but not used

dfsp-spirit · 2022-08-24T14:59:15Z

syncopy/spikes/psth.py

+         onset,
+         trl_end,
+         chan_unit_combs=None,
+         tbins=None,
         samplerate=1000):


not part of this changeset, but the docstring for the samplerate parameter is missing

tensionhead · 2022-08-24T16:02:47Z

The docstring for the frontend, spike_psth(), lists a boolean parameter vartriallen that does not exist. Do we want to implement this later? Otherwise it should be removed.

I implemented it just now :)

The output parameter for the frontend, spike_psth(), seems to be ignored.

yep this still needs to be done!

- still TODO: 'output' parameter Changes to be committed: modified: syncopy/datatype/base_data.py modified: syncopy/spikes/spike_psth.py

Changes to be committed: modified: syncopy/spikes/psth.py

tensionhead added 10 commits August 2, 2022 09:25

Merge pull request #322 from esi-neuroscience/dev

6e17c3a

Dev merge

CHG: Remove statistical data stub

2569bdb

Changes to be committed: modified: syncopy/datatype/__init__.py deleted: syncopy/datatype/statistical_data.py

Merge pull request #338 from esi-neuroscience/dev

af44b7f

Update from Dev

WIP: PSTH

67f3268

- got a working frontend call Changes to be committed: modified: syncopy/datatype/continuous_data.py modified: syncopy/spikes/compRoutines.py modified: syncopy/spikes/psth.py modified: syncopy/spikes/spike_psth.py

WIP: psth frontend

5db155b

- fixed some remaining indexing issues - correct channel labels now Changes to be committed: modified: syncopy/datatype/continuous_data.py modified: syncopy/spikes/compRoutines.py modified: syncopy/spikes/psth.py modified: syncopy/spikes/spike_psth.py

WIP: psth, enabled standard trial averaging

0848bc3

- now it's time to make sense out of the histograms Changes to be committed: modified: syncopy/spikes/compRoutines.py modified: syncopy/spikes/spike_psth.py

WIP: Respect selections

8646953

- should be investigated why the manual construction of the Indexer to access selected trials outside the CR was necessary Changes to be committed: modified: syncopy/spikes/compRoutines.py modified: syncopy/spikes/psth.py modified: syncopy/spikes/spike_psth.py

NEW: Added minimal functional test

b0655e5

Changes to be committed: modified: syncopy/spikes/psth.py new file: syncopy/tests/test_spike_psth.py

tensionhead requested a review from dfsp-spirit August 23, 2022 15:52

dfsp-spirit self-assigned this Aug 24, 2022

FIX: Awkward typo

da7224a

Changes to be committed: modified: syncopy/tests/synth_data.py

dfsp-spirit reviewed Aug 24, 2022

View reviewed changes

syncopy/spikes/spike_psth.py Outdated Show resolved Hide resolved

dfsp-spirit reviewed Aug 24, 2022

View reviewed changes

tensionhead added 2 commits August 24, 2022 18:05

WIP: vartriallen and PR fixes

08b9a26

- still TODO: 'output' parameter Changes to be committed: modified: syncopy/datatype/base_data.py modified: syncopy/spikes/spike_psth.py

FIX: Add samplerate to docstring

2122a46

Changes to be committed: modified: syncopy/spikes/psth.py

tensionhead merged commit 55934ac into dev Aug 24, 2022

tensionhead mentioned this pull request Aug 24, 2022

Implement PSTH method #295

Closed

tensionhead deleted the psth branch September 1, 2022 09:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PSTH #339

PSTH #339

tensionhead commented Aug 23, 2022 •

edited by dfsp-spirit

Loading

codecov bot commented Aug 23, 2022 •

edited

Loading

dfsp-spirit Aug 24, 2022 •

edited

Loading

tensionhead Aug 24, 2022

dfsp-spirit Aug 24, 2022

tensionhead Aug 24, 2022

dfsp-spirit commented Aug 24, 2022

dfsp-spirit commented Aug 24, 2022

dfsp-spirit Aug 24, 2022

dfsp-spirit Aug 24, 2022

dfsp-spirit Aug 24, 2022

tensionhead commented Aug 24, 2022

PSTH #339

PSTH #339

Conversation

tensionhead commented Aug 23, 2022 • edited by dfsp-spirit Loading

PSTH 🚀

Author Guidelines

Reviewer Checklist

codecov bot commented Aug 23, 2022 • edited Loading

Codecov Report

dfsp-spirit Aug 24, 2022 • edited Loading

Choose a reason for hiding this comment

tensionhead Aug 24, 2022

Choose a reason for hiding this comment

dfsp-spirit Aug 24, 2022

Choose a reason for hiding this comment

tensionhead Aug 24, 2022

Choose a reason for hiding this comment

dfsp-spirit commented Aug 24, 2022

dfsp-spirit commented Aug 24, 2022

dfsp-spirit Aug 24, 2022

Choose a reason for hiding this comment

dfsp-spirit Aug 24, 2022

Choose a reason for hiding this comment

dfsp-spirit Aug 24, 2022

Choose a reason for hiding this comment

tensionhead commented Aug 24, 2022

tensionhead commented Aug 23, 2022 •

edited by dfsp-spirit

Loading

codecov bot commented Aug 23, 2022 •

edited

Loading

dfsp-spirit Aug 24, 2022 •

edited

Loading