klabhub / neurostim

Design and run visual neuroscience experiments using Matlab and the Psychophysics Toolbox.
MIT License
5 stars 4 forks source link

Proposed fix for issue #190 (re: jittered parameters not updating) #191

Closed cnuahs closed 2 years ago

cnuahs commented 2 years ago

This change proposes to fix issue #190 wherein parameters that are directly assigned a jitter object (actually any adaptive object) are not updated between trials.

In #188 we decoupled the updating of adaptive parameters from beforeTrial by placing the update code in the code that sets the design specs. We did that so that the update automatically happened before all the other plugins' beforeTrial code (which potentially depend on the adaptive parameter). Importantly, any beforeTrial method defined in an adaptive object will be called along with all the other beforeTrial methods, respecting the plugin order.

That all works well for adaptive objects that are assigned as part of a design object BUT the update code was no longer being called for adaptive objects that were assigned directly to parameters outside of a design.

After discussion with @adammorrissirrommada, we here propose to address this issue by treating adaptive parameters a bit more like the way we handle parameters that are assigned neurostim function strings. Specifically, in cic.run() we locate all neurostim parameters that have been directly assigned adaptive objects and move those adaptive objects to the design object(s).

This allows the user the convenience of assigning adaptive objects directly to parameters if they want that parameter 'adapted' in all conditions, but ensures that all adaptive parameters are updated before all other beforeTrial methods, as required.

Importantly, an adaptive object is NOT added to the design spec if the corresponding plugin.param combination is already part of the design. In effect, assignment by the user within a design trumps any direct assignment.

cnuahs commented 2 years ago

Another upside of this approach is that it removes the need for uplus (e.g., +plg.prop) when using parameters that are directly assigned an adaptive object, because those assignment are "undone" at runtime.

adammorrissirrommada commented 2 years ago

I've reviewed this and it looks good to me. I am keen to remove the +o.prm dereferencing of adaptives. I got tripped up by it when I tried to jitter the size of the dots.

adammorrissirrommada commented 2 years ago

The "trumping" @cnuahs refers to is consistent with the jitter behaviour being the parameter's default value, consistent with normal value assignment before designs and conditions are spec'd out.