search

bachlab / PsPM

A matlab suite for Psycho-Physiological Modelling
GNU General Public License v3.0
45 stars 11 forks source link

new function pspm_combine_markers #538

Closed dominikbach closed 1 year ago

dominikbach commented 1 year ago

Fixes #84.

Changes proposed in this pull request:

This now allows users to use the GLM option "markervalues" to create onsets definition from channels distributed across multiple channels.

dominikbach commented 1 year ago

Hi @teddychao. The function is tested on real data. Could you implement in GUI as part of the "preprocessing" functions?

teddychao commented 1 year ago

@dominikbach Thanks for writing the function. I just have one minor function feature to comment. I am not sure if it is of interest to allow users to save another file that has the combined marker channels? Currently it only allows operations to the original file. If you think this is not unnecessary I can write it there. Also I am not sure if you would like me to add warnings at some lines of code, for example line 77? I am adding it to the GUI now...

dominikbach commented 1 year ago
  1. Current policy is: new channel if data are changed but duration is the same; new file if duration is changed. Here, duration is the same.
  2. Could you implement in GUI before merging.
  3. In line 77, if sts ~= 0 then there will already have been a warning from pspm_load_data; so no second warning is needed.

@teddychao

teddychao commented 1 year ago

Hi @dominikbach , thanks for the feedbacks. I have completed implementing combine_markerchannels to the UI. I am unsure if you would like to have a look at the UI and let me know if you are happy with it, before I merge it to the develop branch? I am also confused about when "new file if duration is changed" happens.. As I understand, combine_markerchannels will combine all the marker channels of the specified data file. This is a static process and I am unsure why duration can be changed? In the test function, I have tested the two channel_action options to make sure replace also works. I made minor changes to your version of combine_markerchannels.m itself by reindenting it according to MATLAB's default indent style, apologies.

dominikbach commented 1 year ago

"New file if duration is changed" - this happens, for example, in pspm_trim or pspm_split_sessions.

dominikbach commented 1 year ago

@teddychao

  1. options.marker_chan_num is not visible in GUI
  2. Why did you put under "tools" rather than under "preprocessing" functions as initially suggested?
dominikbach commented 1 year ago

@teddychao

  1. options.marker_chan_num is not visible in GUI
  2. Why did you put under "tools" rather than under "preprocessing" functions as initially suggested?
teddychao commented 1 year ago

Hi @dominikbach Apologies I did not notice the guidance of location for pspm_combine_markerchannels. I intuitively feel it looks like a processing tool so I put it there. I have now moved it to preparation. I have also added marker_chan_num. It currently will combine all marker channels as default. It also allows selecting specific marker channels if users go to that menu.