YaleDecisionNeuro / PsychTaskFramework

2 stars 4 forks source link

Run most / all phases with `PhaseConfig` and `phase_generic.m` #26

Open shippy opened 7 years ago

shippy commented 7 years ago

There's a finite number of variables in the various stages of task drawing. In theory, a well-defined object/stage in settings.objects.(x) could be drawn and timed in a general boilerplate code. If this could be made sufficiently lightweight, it would remove the need to write custom drawX functions.

The complexity here could get ugly fast. The possible properties would have to be well-spec'd and strictly bounded; at some point, it might be easier to just provide boilerplate code for specific kinds of task parts.

shippy commented 7 years ago

The work in #37 is actually doing exactly this. To quote the relevant comment:

Each phase draws many elements and one action. The draw scripts are fully defined by trial/block settings; they don't need to know about other drawn objects and it is the responsibility of the user to set them up correctly. The action is (in our current use cases) either to wait for some amount of time before moving onto the next phase, or to collect a response.

Actions are exactly what the callback logic was written for. The only problem: how do they know what phase they should be getting settings from? Some phaseSettings struct, or some way to extract the relevant phaseSettings out of blockSettings, is necessary.

PhaseConfig structs are defined by the following:

This structure means that you can basically fully define all four currently implemented tasks with the following cell arrays, and run their trials purely by iterating over them rather than presuming the structure that runTrial currently presumes:

function [ phases ] = getPhasesRA()
phases = { ...
  phaseConfig('showChoice', 6.5, @phase_showChoice, @action_display),...
  phaseConfig('response', 3, @phase_promptResponse, @action_collectResponse), ...
  phaseConfig('feedback', 0.5, @phase_feedback, @action_display), ...
  phaseConfig('ITI', NaN, @phase_ITI, @action_display)};
end

function [ phases ] = getPhasesMDM()
phases = { ...
  phaseConfig('showChoice', 6.5, @phase_showChoice, @action_display),...
  phaseConfig('response', 3, @phase_promptResponse, @action_collectResponse), ...
  phaseConfig('feedback', 0.5, @phase_feedback, @action_display), ...
  phaseConfig('ITI', NaN, @phase_ITI, @action_display)};
end

function [ phases ] = getPhasesSODM()
phases = { ...
  phaseConfig('showChoice', 10, @phase_showChoice, @action_collectResponse), ...
  phaseConfig('feedback', 0.5, @phase_feedback, @action_display), ...
  phaseConfig('ITI', 2, @phase_ITI, @action_display)};
end

function [ phases ] = getPhasesUVRA()
phases = { ...
  phaseConfig('showChoice', Inf, @phase_showChoice, @action_collectResponse), ...
  phaseConfig('feedback', 0.5, @phase_feedback, @action_display), ...
  phaseConfig('ITI', 2, @phase_ITI, @action_display)};
end

function [ phases ] = getPreBlockPhases()
phases = {phaseConfig('preBlock', 'runScript', @phase_preBlock)};
% Default to infinite duration, waiting for break
% ...or it could be defined purely with a draw function
phases = {phaseConfig('preBlock', 'drawCmds', @draw_preBlock)};
end
shippy commented 7 years ago

The question that I worry about is: is this over-engineered? Will it be robust? Copy-pasting tasks is annoying and unsustainable, but this does make the abstraction tree kinda tall:

task -> block -> trial -> phase -> elements to draw, actions to take

Upside: This makes it very easy to switch around which phase collects responses, switching the phase order, go straight to re-drawing elements, and so on. It bodes well for versatility and interoperability, if it is well-documented. Downside: It might be challenging to document the innards well enough that a reasonably competent researcher can debug the process if something goes wrong.

This will definitely problematize the cookbook (#62). It's yet another approach to get things done. At this point, in decreasing level of removedness from code, we have:

  1. Write your task with phase abstractions, re-using & configuring existing draw & action scripts. No need to write any trial or phase scripts -- just specify in drawCmds and action whatever you need.
  2. Re-use phase scripts fully, but specify what callbacks they should have in XYZ_runTrial (and have phase scripts construct their ad-hoc phase configs before the callback).
  3. Write those phase scripts that draw slightly different elements and/or take slightly different action than the general-library phase scripts. Re-use some draw scripts and/or action scripts, and re-use runTrial and runBlock. (see: current version of tasks/UVRA)
  4. Write separate trial and phase scripts for each task (never did that)
  5. Write separate block scripts for each block (see: initial_RA branch).

The good thing is that I think this downgrades cleanly. If someone doesn't want to use phase abstractions, that should be okay -- even if general phase scripts use them on the inside, that shouldn't be a bother. runBlock and runTrial have been untouched for a while now; using the phase specification of a task trial would require shaking up runTrial, but that can be done by creating a new trial function (runPhaseAbstractions?) and letting tasks keep runTrial as their default handle.

shippy commented 7 years ago

Problems / ugly edge cases!

Problem 1: phase_X has to know how to fill in a NaN / absent duration. One possible solution (and it doesn't have to assume this is just an ITI issue -- it might try to fill in from trialData and blockSettings.durations.(phaseName) if it doesn't know):

if isnan(phases{end}.ITI) phases{end}.duration = trialData.ITIs;

Problem 2: If game.constantTrialDuration is set, then it has to know to calculate durations elapsed thus far -- but if this is the structure in which durations are stored, then they might be a bit difficult to extract?

  extractMaxDurations = sum(cellfun(@(p) p.duration, phases), 'omitnan')

...and the summing function should rule out any values passing isinf.

Edit: Potential solution to Problem 2 is to generate trial duration rather than ITI durations. Then, we can just look at the first trial timestamp and keep the ITI on until the time elapses.

shippy commented 7 years ago

HLFF feedback phase is currently the proof of concept for this - things seem to run pretty well. HLFF trials are defined by the following config:

s.game.trialFn = @runGenericTrial;
% See lib/phase/phaseConfig.m for meaning
s.game.phases = { ...
  phaseConfig('showChoice', Inf, @phase_showChoice, @action_collectResponse), ...
  phaseConfig('feedback', 0.5, @phase_generic, @action_display, @drawFeedback), ...
  phaseConfig('ITI', 2, @phase_ITI, @action_display)};
s.game.referenceDrawFn = @drawRef;

At some point, I'll create an experimental branch and implement this completely for at least one task; for now, moving this out of the Essential features milestone.