Skip to content

add new HPR-reward model#729

Merged
dominikbach merged 4 commits intodevelopfrom
hpr-reward
Jun 24, 2024
Merged

add new HPR-reward model#729
dominikbach merged 4 commits intodevelopfrom
hpr-reward

Conversation

@dominikbach
Copy link
Copy Markdown
Contributor

Fixes #579 .

Changes proposed in this pull request:

  • add new basis function pspm_bf_hprf_rew
  • add new GLM to pspm_init
  • add new GLM to GUI

@dominikbach dominikbach self-assigned this Jun 20, 2024
@dominikbach dominikbach requested a review from teddphil June 20, 2024 08:22
@dominikbach dominikbach added the Completed & Waiting for Review Completed and waiting for review label Jun 20, 2024
@dominikbach dominikbach added this to the v7.0 milestone Jun 20, 2024
Comment thread src/pspm_cfg/pspm_cfg_glm_hp_rew.m Outdated
hprf_rew.name = 'HPRF_REW';
hprf_rew.tag = 'hprf_rew';
hprf_rew.val = {1};
hprf_rew.help = {'HPRF_REW without derivatives.'};
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I suggest to include the full name of "HPRF_REW" in the help text for people like me who do not have knowledge of the field but may have the intention to use it as a black box for own purposes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

good idea - fixed.

vars.glmhelp = '';

% load default settings
glm_hp_rew = pspm_cfg_glm(vars);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

There is a "Nuisance File" option, and it does not have to be specified, because on the current design. I am unsure if this is intentional or not. If it is an optional file, I would suggest to add "optional" in the help text. If it is mandatory, I suggest to report an error if it is not suggested. Currently it is not mandatory and can be left black for running the module.

image

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

a generic stylistic issue for GLM, will be dealt with in future PR.

glm_hp_rew.tag = 'glm_hp_rew';

% set callback function
glm_hp_rew.prog = @pspm_cfg_run_glm_hp_rew;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

There are errors that appear in pspm_cfg_data_design_selector, but they have been there before implementing #728 . I will mark the errors in that pull request #728.
image

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

OK

@teddphil
Copy link
Copy Markdown

Minor comments regarding the text and UI. Critical errors will be put in #728 since they are more related to CFG functions than this new model.

@teddphil teddphil 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 Jun 24, 2024
@dominikbach dominikbach merged commit 0852a8a into develop Jun 24, 2024
@dominikbach dominikbach deleted the hpr-reward branch June 24, 2024 16:06
@teddphil teddphil removed the Approved The pull request has been approved and can be checked and then merged. label Aug 5, 2024
@teddphil teddphil mentioned this pull request Oct 12, 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.

Implement new HPR model

2 participants