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

Fix issue 503 --- pspm_sf relevant functions #502

Merged
merged 4 commits into from
Aug 11, 2023
Merged

Conversation

teddychao
Copy link
Contributor

@teddychao teddychao commented Jul 17, 2023

Introduction

Issue #486: Users reported an issue when loading data and running SF analysis.
This pull request attempts to fix the issue #503 for pspm_sf and its relevant functions. Users have reported that pspm_sf will report errors when processing data whose time_unit is defined as marker. A series of issues are also discovered and marked during investigating this issue. The results are function-specifically listed below for discussing.

Method

pspm_sf should be updated in line with the last update, i.e. pspm_sf_dcm should accept model and options.

Results

GUI

SF model should allow missing epochs as an optional field. Now it looks like below.
image
In this window, missing epochs is an optional field to add. Users can add missing epochs if they need and ignore missing epochs as default if they do not need to. This part is done by updating the functions pspm_cfg_sf and pspm_cfg_run_sf.

src/pspm_cfg/pspm_cfg_run_sf.m

  • Line 81--83, adding transferring missing data to the function from GUI.

src/pspm_cfg/pspm_cfg_sf.m

  • Line 196: the default value for marker_chan_num is set to 1.
  • Line 299--319: the missing epoch can be set to none in the GUI if preferred.

src/pspm_options.m

-- Line 582: int should include 0 as some values need to include 0 as an acceptable value. Further refinement can be done for some parameters by adding more field checking code in the corresponding functions.

src/pspm_overwrite.m

  • Line 99--100: There should be a default return value as pspm_overwrite is not expected to be a void function.

src/pspm_sf.m

  • Line 169 etc
    • iSn is replaced by iFile as it denotes the number of datafile.
  • Line 204
    • It is more robust to specify the corresponding .overwrite in the code.
  • Section 3.6, Line 244--252
    • The logic of loading data based on the setting of options.marker_chan_num has been updated.
      • model.datafile is now loaded from the corresponding session iFile
    • events will be loaded based on the loaded ndata.
  • Line 264
    • epochs has been updated to the corresponding iFile session.

src/pspm_sf_auc.m

pspm_sf_auc should be in line with pspm_sf_dcm in terms of its input.

src/pspm_sf_mp.m

  • pspm_sf_mp should be in line with pspm_sf_dcm in terms of its input.
  • Line 172--174
    • The initial values of a, asp and ind have been set up.

src/pspm_sf_scl.m

pspm_sf_scl should be in line with pspm_sf_dcm in terms of its input.

@teddychao teddychao self-assigned this Jul 17, 2023
@teddychao teddychao added the Help Wanted Extra attention is needed label Jul 17, 2023
@teddychao teddychao added this to the v6.1 milestone Jul 17, 2023
@teddychao teddychao marked this pull request as ready for review July 17, 2023 14:13
@teddychao teddychao added Completed & Waiting for Review Completed and waiting for review and removed Help Wanted Extra attention is needed labels Jul 24, 2023
@teddychao teddychao changed the title Update pspm_sf relevant functions Fix issue 503 -- pspm_sf relevant functions Jul 24, 2023
@teddychao teddychao changed the title Fix issue 503 -- pspm_sf relevant functions Fix issue 503 --- pspm_sf relevant functions Jul 24, 2023
@teddychao teddychao linked an issue Aug 7, 2023 that may be closed by this pull request
Copy link
Contributor

@dominikbach dominikbach left a comment

Choose a reason for hiding this comment

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

There is one question; I also left this question in the other pull request.

src/pspm_cfg/pspm_cfg_sf.m Show resolved Hide resolved
@dominikbach
Copy link
Contributor

Apparently all changes were already commited with another merge request. Can the request be closed?

@dominikbach dominikbach added Solved and removed Completed & Waiting for Review Completed and waiting for review labels Aug 11, 2023
@teddychao
Copy link
Contributor Author

Apparently all changes were already commited with another merge request. Can the request be closed?

Yes I will do now.
I did not close this because I have not tested the question you mentioned. I tested the data from our user and confirmed that there is no issue when running the SF analysis with the current specifications.

@teddychao teddychao merged commit 0d635bf into develop Aug 11, 2023
1 check passed
@teddychao teddychao deleted the SF_marker_issue branch August 11, 2023 18:22
@teddychao teddychao removed the Solved label Feb 26, 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.

pspm_sf has issues when processing data whose unit is defined by marker Problems using SF
2 participants