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 updating of adaptive plugins re: commit 6b6e581. #188

Merged
merged 2 commits into from
Nov 30, 2021

Conversation

cnuahs
Copy link
Contributor

@cnuahs cnuahs commented Nov 27, 2021

Commit 6b6e581 modified the adaptive base class to
call it's update() method in beforeTrial rather than
afterTrial. However, beforeTrial is only called on
plugins after beforeTrial is called on the current
block/design object. As a result, the design object
cannot know the new/updated value that should be
assigned to a parameter for the next trial. The design
object therefore (re-)assigns the value from the previous
trial.

This change calls beforeTrial on any adaptive plugins
before calling beforeTrial on the current block/design.

Commit 6b6e581 modified the adaptive base class to
call it's update() method in beforeTrial rather than
after trial. However, beforeTrial is only called on
plugins *after* beforeTrial is called on the current
block/design object. As a result, the design object
cannot know the new/updated value that should be
assigned for the next trial.

This change calls beforeTrial on any adaptive plugins
before calling beforeTrial on the current block/design.
@adammorrissirrommada
Copy link
Member

Ahhhh, this is difficult stuff, but this makes sense to me. This is essentially like it was when it was in afterTrial() except that the trial counter has (appropriately) ticked over to the next (current) trial. This solution, like the original one, assumes that the calculation of the new value for adaptive does not depend on the current values of other plugins. In other words, the assignment of condition values in the design class doesn't respect c.order.

An alternative solution might be to make a distinct update function in the adaptive class that is called by the design class at the time of assignment of new values for the current trial/condition. That would leave beforeTrial() available for its traditional use and position in the trial loop sequence (i.e. custom behaviour after the condition spec has been fully set out across all plugins).

@cnuahs
Copy link
Contributor Author

cnuahs commented Nov 29, 2021

Hmm, ok... effectively renaming the existing beforeTrial() method in the adaptive base class to baseUpdate() or something, have the block object call that in it's beforeTrial() before calling getValue(), and leave the adaptive base class with no beforeTrial() method... child classes being free to add one if appropriate. Calling of beforeTrial() (if defined in a child class) would respect c.pluginOrder. Calling baseUpdate() would occur in whatever order conditions are listed in the block object...

Is that how you picture it?

@adammorrissirrommada
Copy link
Member

That's what I had in mind, yep. Whether it is a robust solution or not, not entirely sure. Perhaps @bartkrekelberg can confirm whether he thinks this is OK before we merge it.

@bartkrekelberg
Copy link
Member

I like this second solution.
If I understand it correctly, by putting the update in the code that sets the specs, it automatically moves before all the other plugins' beforeTrial code. That seems cleaner than rearranging the pluginOrder in the block. beforeTrial code to achieve the same result.

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.

3 participants