[ENH]: review debug mode skip sync test

[marcobarilari]

Is there an existing issue for this?

• [X] I have searched the existing issues

New feature

Opening the discussion about debug mode and wether to skip the sync test or not

Now debug mode is currently providing:

   % init PTB with different options in concordance to the debug Parameters

    if cfg.debug.do

        Screen('Preference', 'SkipSyncTests', cfg.skipSyncTests);
        Screen('Preference', 'Verbosity', 0);
        Screen('Preference', 'SuppressAllWarnings', 1);

        fprintf('\n\n\n\n');
        fprintf('########################################\n');
        fprintf('##   DEBUG MODE. TIMING WILL BE OFF.  ##\n');
        fprintf('########################################');
        fprintf('\n\n\n\n');

        testKeyboards(cfg);

    end

    if cfg.debug.transpWin
        PsychDebugWindowConfiguration;
    end

skip sync test as it is, does not change between debug mode or not. Either we skip this double setting here or we have that in debug mode we actually skip the sync test by default (because we are testing on our macs etc.)

pro to skip: on macs (at least the 'old' ones in which PTB is still half working) it usually fails so for debugging you don't want to set 1 manually

cons to skip: ???

[marcobarilari]

Well, noticing now that it also print a message that implies that debug mode is skipping so I would hard code

Screen('Preference', 'SkipSyncTests', 1);

to match it

[Remi-Gau]

Well, noticing now that it also print a message that implies that debug mode is skipping so I would hard code

Screen('Preference', 'SkipSyncTests', 1);

to match it

Yes let's make sure that we skip the sync test when in debug mode indeed.

[Remi-Gau]

So in theory the debug mode should be set SkipSyncTests to 2 when in debug mode by checkCppPtbCfg

https://github.com/cpp-lln-lab/CPP_PTB/blob/bf19773c174d0b60279da0804a8eeb1a40521f96/src/defaults/checkCppPtbCfg.m#L39

[marcobarilari]

That could work but in theory if I set cfg.skipSyncTests = 0; before running my experiment I am over writing the default so it will change nothing in the end.

When I run the code for theactual experiment I will set cfg.skipSyncTests = 0; because I have to so this info id in the main script, everytime I want to test the code on my computer I have to either set cfg.skipSyncTests = 1 (or 2); or delete this line.

Hard coding it will avoid this problem. Unles instead of cfg.skipSyncTests = 2; you use cfg.skipSyncTestsDebug = 2;`

Does it make sense?

[Remi-Gau]

OK I am trying to have those tests

function test_checkCppPtbCfg_basic()

    % set up
    cfg = checkCppPtbCfg();

    % test data
    expectedCfg = cppPtbDefaults('all');
    expectedCfg.eyeTracker.do = false;
    expectedCfg.skipSyncTests = 1;

    % test
    assertEqual(expectedCfg, cfg);

end

function test_setDefaultsPtb_no_debug()

    % set up
    cfg.debug.do = false;
    cfg = checkCppPtbCfg(cfg);

    % test data
    expectedCfg = cppPtbDefaults('all');

    % make sure that  debug and skipSyncTests have the right value
    expectedCfg.debug.do = 0;    
    expectedCfg.skipSyncTests = 0;

    % test
    assertEqual(expectedCfg, cfg);

end

The issue with the PR on the localizer is that we also have this that probably should not be there:

https://github.com/Remi-Gau/localizer_visual_motion/blob/eb48cd3e1fd28e7225a1f33c16c78d0aececffda/subfun/defaults/checkParameters.m#L33

[Remi-Gau]

OK and this is what I have for the visual localizer now:

function test_checkParameters_no_debug_fullscreen()

    % set up
    cfg.design.localizer = 'MT';
    cfg.debug.do = false;
    cfg.debug.transpWin = 0;
    cfg.debug.smallWin = 0;     

    cfg = checkParameters(cfg);

    % prepare exected results
    cfg = removeDirFieldForGithubAction(cfg);

    load(fullfile(fileparts(mfilename('fullpath')), 'data', 'config_MT.mat'), 'expected');

    expected.debug.do = false;
    expected.debug.transpWin = 0;
    expected.debug.smallWin = 0;    
    expected.skipSyncTests = 0;

    % test
    assertEqual(cfg.debug, expected.debug);
    assertEqual(cfg.skipSyncTests, expected.skipSyncTests);

end

function test_checkParameters_no_debug()

    % set up
    cfg.design.localizer = 'MT';
    cfg.debug.do = false;

    cfg = checkParameters(cfg);

    % prepare exected results
    cfg = removeDirFieldForGithubAction(cfg);

    load(fullfile(fileparts(mfilename('fullpath')), 'data', 'config_MT.mat'), 'expected');

    expected.debug.do = false;
    expected.debug.transpWin = 1;
    expected.debug.smallWin = 1;    
    expected.skipSyncTests = 0;

    % test
    assertEqual(cfg.debug, expected.debug);
    assertEqual(cfg.skipSyncTests, expected.skipSyncTests);

end

function test_checkParameters_debug()

    cfg.design.localizer = 'MT';
    cfg.debug.do = true;

    cfg = checkParameters(cfg);

    % prepare exected results
    cfg = removeDirFieldForGithubAction(cfg);    

    load(fullfile(fileparts(mfilename('fullpath')), 'data', 'config_MT.mat'), 'expected');

    expected.debug.do = true;
    expected.debug.transpWin = 1;
    expected.debug.smallWin = 1;
    expected.skipSyncTests = 1;

    % test
    assertEqual(cfg.debug, expected.debug);
    assertEqual(cfg.skipSyncTests, expected.skipSyncTests);

end