Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update channel loading logic in GUI #666

Merged
merged 35 commits into from
Apr 5, 2024
Merged

Update channel loading logic in GUI #666

merged 35 commits into from
Apr 5, 2024

Conversation

dominikbach
Copy link
Contributor

@dominikbach dominikbach commented Mar 18, 2024

Fixes #609 .

Changes proposed in this pull request:

  • new unified function for channel selection in matlabbatch: pspm_cfg_channel_selector
  • implemented in the following GUI modules:

Single channel selection:

pspm_cfg_dcm
pspm_cfg_find_sounds
pspm_cfg_get_markerinfo
pspm_cfg_glm
pspm_cfg_pp_emg_data
pspm_cfg_pp_heart_data
pspm_cfg_pp_scr
pspm_cfg_sf
pspm_cfg_trim
pspm_cfg_resp_pp
pspm_cfg_split_sessions
pspm_cfg_sf
pspm_cfg_interpolate

Modality-agnostic channel selection

pspm_cfg_artefact_rm
pspm_cfg_extract_segments

Eyetracker channel selection

pspm_cfg_pupil_correct
pspm_cfg_pupil_preprocess
pspm_cfg_find_valid_fixations
pspm_cfg_gaze_convert
pspm_cfg_pupil_size_convert

multiple channels

pspm_cfg_combine_markerchannels

  • removed unused functions pspm_downsample and associated GUI items

  • removed unused option "simple SCR artefact removal" from pspm_cfg_artefact_rm

  • removed superseded GUI item pspm_cfg_data_convert
    --

  • fixed some bugs

@dominikbach dominikbach added the In Progress Currently being worked on label Mar 18, 2024
@dominikbach dominikbach self-assigned this Mar 18, 2024
@teddychao
Copy link
Contributor

Questions that are mentioned on the PsPM meeting

  • find_valid_fixations
    • Two options fields, eyes and newfile, are used by the GUI but not used by the function.
  • glm
    • For sps, if we use sps to specify model.channel, pspm_glm will not report errors. This has been checked by the test function now. I am uncertain about the issue described at Bug in GUI for SPS GLM #607 about whether it comes from the GUI or the function.

This was referenced Mar 18, 2024
@dominikbach dominikbach added the Completed & Waiting for Review Completed and waiting for review label Mar 21, 2024
@dominikbach dominikbach added this to the v6.2 milestone Mar 21, 2024
@dominikbach dominikbach removed the In Progress Currently being worked on label Mar 21, 2024
@teddychao teddychao added Under Review and removed Completed & Waiting for Review Completed and waiting for review labels Mar 22, 2024
@teddychao
Copy link
Contributor

teddychao commented Apr 3, 2024

Corrections

  1. src/ext/matlabbatch/cfg_util.m is converted into LF to make it in line with remaining functions.
  2. src/pspm_cfg/pspm_cfg_artefact_rm.m variable names changed to be lowercases to make it in line with other functions.
  3. In src/pspm/cfg/pspm_cfg_extract_segments.m, at line 187, timeunits should be timeunit.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this file and pspm_convert_ecg2hb, channel was set up as an independent variable, whilst in pspm_convert_ecg2hb_amri, channel is a field in options. It could be written intentionally in this way. I do not know if it is good to make these consistent.

@@ -285,10 +204,10 @@
ppg2hb_method.help = {['Convert the PPG data into heart rate by using the selected method.']};

ppg2hb = cfg_exbranch;
ppg2hb.name = 'Convert peripheral pulse oximetry to Heart Beat';
ppg2hb.name = 'Convert peripheral pulse oximetry (PPG) to Heart Beat';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I add (PPG) here because "peripheral pulse oximetry" is not a very common name.

end
end
scr_pp_options.channel_action = job.chan_action;
[sts, output] = pspm_scr_pp(scr_pp_datafile, scr_pp_options, scr_pp_options.channel);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I correct this file by adding scr_pp_options.channel. In pspm_scr_pp, channel is an independent variable from options.

@teddychao
Copy link
Contributor

teddychao commented Apr 3, 2024

List of issues that I am unsure how to resolve

SF

image

The settings above leads to undefined mrk_chan in pspm_cfg_run_sf.

Extract segments

In the file pspm_cfg_run_extract_segments, at line 51--55, there is a switch loop where mode can be either auto or manual. mode is the first variable of pspm_extract_segments. However, in the function itself, the first variable is method. In the help text of pspm_extract_segments, method is not described In arguments, and I assume it is actually mode there. Yet, mode can be either file, data or model, so auto or manual is not included here. I do not know what is expected here.

Copy link
Contributor

@teddychao teddychao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @dominikbach Please have a look at: #666 (comment) and merge if you are happy with the current GUI test results. There are some issues #666 (comment)

@@ -3,7 +3,7 @@
cfg.name = 'Tools';
cfg.tag = 'tools';
cfg.values = {pspm_cfg_display, pspm_cfg_rename, pspm_cfg_split_sessions, ...
pspm_cfg_merge, pspm_cfg_artefact_rm, pspm_cfg_downsample, pspm_cfg_interpolate, ...
pspm_cfg_merge, pspm_cfg_artefact_rm, pspm_cfg_interpolate, ...
pspm_cfg_extract_segments, pspm_cfg_segment_mean, pspm_cfg_get_markerinfo, ...
pspm_cfg_data_editor, pspm_cfg_data_convert};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pspm_cfg_data_convert does not exist. Shall we create it or remove convert data from Tools? I am confused about why it is there.

@teddychao teddychao added Changes Requested Authors are requested to make changes and removed Under Review labels Apr 3, 2024
@dominikbach dominikbach added Completed & Waiting for Review Completed and waiting for review and removed Changes Requested Authors are requested to make changes labels Apr 5, 2024
@teddychao teddychao added Approved The pull request has been approved and can be checked and then merged. and removed Completed & Waiting for Review Completed and waiting for review labels Apr 5, 2024
@teddychao teddychao merged commit 5c12540 into develop Apr 5, 2024
1 check passed
@teddychao teddychao deleted the gui-channel-update branch April 5, 2024 14:31
@teddychao teddychao removed the Approved The pull request has been approved and can be checked and then merged. label Apr 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update logic of eyetracking preprocessing
2 participants