klabhub / neurostim

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

Adaptive #63

Closed bartkrekelberg closed 7 years ago

bartkrekelberg commented 7 years ago

@cnuahs and @adammorrissirrommada , I created this adaptive branch to create a more structured approach to adaptive parameters.

Currently the master branch has cic.jitter , staircase and nDown1Upstaircase, and the Quest related parameters. In essence they all do the same thing (change a parameter across trials). Logging of these changes was not consistent and parameters could not be set differently in different conditions (e.g. different jitter in one conditionn, or a two independent staircases). To address this I created one adaptive class and three children (nDown1UpsStaircase, Jitter and Quest); logging of the parameters is now automatic.

I also changed where these kind of parameters need to be defined. Because they apply to conditions, I think they belong with the factorial specification. This obviously breaks some old scripts (I adjusted the demos already), but the changes are minor.

Adding other adaptive procedures now also becomes quite simple as most of the functionality is in the abstract Adaptive class. The logic of the adaptation is still in the derived classes and I just copied that from the classes Shaun (staircase), Teresa (Jitter), and I (Quest) made previously.

Can you please check this branch, confirm that it works for what you want to do and comment on possible additions/changes we may want at this stage?

I 'd like to pull it back into the master quite soon, before they start diverging in other ways...

bartkrekelberg commented 7 years ago

@cnuahs & @adammorrissirrommada To see how adapative parameters can be used, look at the adaptiveDemo in the demos directory

adammorrissirrommada commented 7 years ago

Hi Bart,

We agree completely with this approach and had come to the same conclusion a couple of weeks back.

One suggested change:

we thought that CIC shouldn't have to worry itself with checking whether the specs contain an adaptive object, That should happen either in the bock class' get.condition(o) or, there could be a tiny condition class that just holds the unique conjunction of plugin/prop values for a given condition, and evaluates the adaptive object if needed, such that everyone else just receives the values for this trial.

Perhaps even better would be that cic doesn't worry itself about setting the props at all. It just says "next trial" and the block class reacts and calls the "go" function for the appropriate condition object, which in turn sets all the new prop values itself without passing anything back up the chain (or this last part could just happen in the block class if a separate condition class is overkill).

Any thoughts on this kind of approach?

Obviously this is just making it pretty, so it's not urgent or essential.

bartkrekelberg commented 7 years ago

I agree that the checking for the adaptive object is kind of ugly. The main reason to do it there (and not in a block or condition) is that it allows me to create appropriate duplicates of the adaptive parameter (e.g. one staircase for each condition). That is something a block or condition class would not necessarily know about. This is not unique to the adaptive class parameters, but all parameters that are expanded from singleton to "all conditions". I kind of like that feature ....

On Fri, Nov 18, 2016 at 9:31 AM Adam Morris notifications@github.com wrote:

Hi Bart,

We agree completely with this approach and had come to the same conclusion a couple of weeks back.

One suggested change:

we thought that CIC shouldn't have to worry itself with checking whether the specs contain an adaptive object, That should happen either in the bock class' get.condition(o) or, there could be a tiny condition class that just holds the unique conjunction of plugin/prop values for a given condition, and evaluates the adaptive object if needed, such that everyone else just receives the values for this trial.

Perhaps even better would be that cic doesn't worry itself about setting the props at all. It just says "next trial" and the block class reacts and calls the "go" function for the appropriate condition object, which in turn sets all the new prop values itself without passing anything back up the chain (or this last part could just happen in the block class if a separate condition class is overkill).

Any thoughts on this kind of approach?

Obviously this is just making it pretty, so it's not urgent or essential.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/klabhub/neurostim-ptb/pull/63#issuecomment-261476026, or mute the thread https://github.com/notifications/unsubscribe-auth/ALbI072a_6SELMdqFBfcpoDjaphRiq_vks5q_WJFgaJpZM4K2Nkd .

adammorrissirrommada commented 7 years ago

I don't really understand what you mean... this line:

specs = c.blocks(c.block).condition;

returns a cell array of plugin/props for the current condition from the block class. The value of any of those can be an adaptive plugin object, but couldn't block have instead called getValue() on the object before returning the specs so that CIC just receives the value? CIC doesn't seem to do anything other than call getValue() if needed as far as I can see.

I suspect I am just not understanding what you mean. The duplicates you refer to are already done somewhere in factorial/block in my mind, not in CIC?

bartkrekelberg commented 7 years ago

Sorry my mistake. There are two places where the code checks for an adaptive object. One in factorial one in cic. I was thinking of the former.

You're right , there could be a condition class with a getValue member. That would be cleaner. But that class would consist only of the 5 lines of ode that are now in CIC.

An alternative is to change block.nextTrial so that it takes a handle to cic and move the code that is now in cic (i.e. the currentspecs code) there. I think that is preferable over creating another class.

I just pushed that change to this branch. Have a look to see if this does what you meant.

adammorrissirrommada commented 7 years ago

Yep, I'm happy with that solution.

Your adaptive thing seems to work for me. The only gripe I have with how we are using factorial is that the inclusion of a jitter, singleton, staircase is entered as if it is a factor, which I think is counterintuitive.

so, for example:

myFac.fac1.fix.X = {-10 0 10};
myFac.fac1.fix.Y = plugins.jitter(c,{0,4},'distribution','normal','bounds',[-5 5]);
myFac.fac2.dots.direction={-90 90}; %Two dot directions

or equivalently:

myFac.fac1.fix.X = {-10 0 10};
myFac.fac2.dots.direction={-90 90};
myFac.fac3.fix.Y = plugins.jitter(c,{0,4},'distribution','normal','bounds',[-5 5]);

What if we instead renamed factorial.m to something like design.m, and it specifies the conditions for a given design. This can include a factorial and/or any number of other property values, including adaptives.

For example:

myDes.fac1.fix.X = {-10 0 10}; myDes.fac2.dots.direction={-90 90}; myDes.fix.Y = plugins.jitter(c,{0,4},'distribution','normal','bounds',[-5 5]);

As another example, suppose I just wanted to have two separate sets of "fixed" values from one block to another. I could create two separate design objects

myDes1.fix.X = -10; myDes1.dots.direction = 90;

myDes2.fix.X = 10; myDes2.dots.direction = -90;

myBlock1=block('myBlock1',myDes1); myBlock2=block('myBlock2',myDes2);

bartkrekelberg commented 7 years ago

Yes that makes sense to me. This will also make it more obvious that you use the design class to define just a single condition (or something that you don't think of as a factorial design).

Of course this takes a fair bit of change under the hood and is not backward compatible. But now is a good time to do it.

I assume you're volunteering, Adam?

B

On Mon, Nov 21, 2016 at 4:43 AM Adam Morris notifications@github.com wrote:

Yep, I'm happy with that solution.

Your adaptive thing seems to work for me. The only gripe I have with how we are using factorial is that the inclusion of a jitter, singleton, staircase is entered as if it is a factor, which I think is counterintuitive.

so, for example:

myFac.fac1.fix.X = {-10 0 10}; myFac.fac1.fix.Y = plugins.jitter(c,{0,4},'distribution','normal','bounds',[-5 5]); myFac.fac2.dots.direction={-90 90}; %Two dot directions

or equivalently:

myFac.fac1.fix.X = {-10 0 10}; myFac.fac2.dots.direction={-90 90}; myFac.fac3.fix.Y = plugins.jitter(c,{0,4},'distribution','normal','bounds',[-5 5]);

What if we instead renamed factorial.m to something like design.m, and it specifies the conditions for a given design. This can include a factorial and/or any number of other property values, including adaptives.

For example:

myDes.fac1.fix.X = {-10 0 10}; myDes.fac2.dots.direction={-90 90}; myDes.fix.Y = plugins.jitter(c,{0,4},'distribution','normal','bounds',[-5 5]);

As another example, suppose I just wanted to have two separate sets of "fixed" values from one block to another. I could create two separate design objects

myDes1.fix.X = -10; myDes1.dots.direction = 90;

myDes2.fix.X = 10; myDes2.dots.direction = -90;

myBlock1=block('myBlock1',myDes1); myBlock2=block('myBlock2',myDes2);

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/klabhub/neurostim-ptb/pull/63#issuecomment-261835535, or mute the thread https://github.com/notifications/unsubscribe-auth/ALbI0-9rwgBBoBJnvPeno_z8-PcawoGbks5rARNdgaJpZM4K2Nkd .

cnuahs commented 7 years ago

For what its worth, the Adaptive branch works well for me. We could go ahead and merge it as is and give some more thought to the idea of a design object as Adam has described...

bartkrekelberg commented 7 years ago

Merging now would mean a backward incompatible change (for jitter) and then we'd get another one soon, with the factorial->design change... This does not affect me (or anyone in my lab) but I don't know how many scripts are out there in your labs that would need to be changed (twice).

I suggest we wait until we have the design change implemented too. Adam do you have time to do it, or should I?

On Sun, Nov 27, 2016 at 3:47 AM Shaun Cloherty notifications@github.com wrote:

For what its worth, the Adaptive branch works well for me. We could go ahead and merge it as is and give some more thought to the idea of a design object as Adam has described...

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/klabhub/neurostim-ptb/pull/63#issuecomment-263098956, or mute the thread https://github.com/notifications/unsubscribe-auth/ALbI0-yY0HG50AE_Nw4HAAyqxXS-Drnsks5rCO9GgaJpZM4K2Nkd .

adammorrissirrommada commented 7 years ago

We had second thoughts about the value of my suggested changes. There's a discussion to be had about what is the best format, but I won't have time to look into this further for another 2 weeks probably... I'm flat out preparing for a couple of conferences, and then have to send this flamin' V1 paper off!!

There aren't many users at this end, so merging twice wouldn't be a big deal for us.

cnuahs commented 7 years ago

I think Adam and I have come to the conclusion that the root of the problem the proposed design class would be trying to solve is the ambiguity surrounding singleton expansion in the factorial class.

At present, singletons are expanded silently behind the scenes, i.e.,

myFac.fac1.fix.X = {-10 0 10}; myFac.fac1.fix.Y = 0;

causes fix.Y to be expanded to be a 1x3 vector, equivalent to,

myFac.fac1.fix.X = {-10 0 10}; myFac.fac1.fix.Y = repmat(0,1,3);

That has been ok up until now because it was pretty safe to assume that that is actually what the user intended. However, the new adaptive class(es) change things in that it is no longer clear what should happen wrt expansion of singleton adaptive objects. Should you get multiple independent adaptive objects or should you multiple references (handles) to the same adaptive object.

As it stands today, the adaptive class overrides repmat() to return deep copies (i.e., duplicates) of the singleton object, so

myFac.fac1.fix.X = {-10 0 10}; myFac.fac1.fix.Y = plugins.jitter(c,{0,4},'distribution','normal','bounds',[-5 5]);

produces three independent jitter objects (or three staircases if I'd used a staircase object in the example). That isn't obvious since this behaviour of repmat (called behind the scenes, and provided by the adaptive class) is actually different to that of repmat for all other classes and data types, and specifically, for other handle classes. When you repmat() a handle object, you usually get duplicate handles to a single object, not deep copies (duplicates) of the object.

One solution, which Adam and I have discussed is to not allow singleton expansion behind the scenes in the factorial class. Instead, force the user to explicitly decide, and specify, if they want multiple independent objects (duplicates, or deep copies), or if they want multiple handles to a single object. For example, the scenario above:

myFac.fac1.fix.X = {-10 0 10}; myFac.fac1.fix.Y = plugins.jitter(c,{0,4},'distribution','normal','bounds',[-5 5]);

would cause an error. Forcing the user to specify either:

myFac.fac1.fix.X = {-10 0 10}; myFac.fac1.fix.Y = repmat(plugins.jitter(c,{0,4},'distribution','normal','bounds',[-5 5]).1,3);

producing multiple handles to a single adaptive object (in this case a jitter object), or

myFac.fac1.fix.X = {-10 0 10}; myFac.fac1.fix.Y = duplicate(plugins.jitter(c,{0,4},'distribution','normal','bounds',[-5 5]),1,3);

producing multiple independent adaptive (i.e., jitter) objects.

The following would also cause an error:

myFac.fac1.fix.X = {-10 0 10}; myFac.fac1.fix.Y = 0;

so there is a backwards incompatibility there. Existing scripts with usage like that would need to be changed to:

myFac.fac1.fix.X = {-10 0 10}; myFac.fac1.fix.Y = repmat(0,1,3);

or maybe

myFac.fac1.fix.X = {-10 0 10}; myFac.fac2.fix.Y = 0;

depending on the context.

This would require a couple of fairly small changes:

  1. remove singleton expansion in the factorial class and raise an error instead,
  2. remove the repmat() method in the adaptive class, and
  3. modify the behaviour of the duplicate() method in the parent plugin class to accept a size argument to produce multiple (deep) copies (so it could be called with a syntax something like the example above).

The advantage of this solution is that the behaviour of repmat() and duplicate() would be consistent across all classes and data types.

Now, the other point raised above wrt the proposed design class is the idea of so called "conditions" which one doesn't think of as part of a factorial design. I think we (Adam and I) concluded that actually this is was poorly conceived. If there is a condition that one want's to specify outside an existing factorial design, it is actually really a second factorial object, whose factor(s) have only one level.

bartkrekelberg commented 7 years ago

At the moment you can write

myFac.fac1.fix.Y = 0 or myFac.fac1.fix.Y = repmat(0,1,3);

I find the first line much easier to understand.Especially given that singleton expansion is standard in Matlab this seems to be close to normal usage. And if you think of it as being expanded then assigning it to part of the fac1 is not that counter-intuitive. It is a variable that happens to have the same value across all levels of fac1.

The current code for adaptive classes assumes that if you write a singleton, you mean a deep copy. I set it up that way because I thought it was the default case (not relevant for jitter, but most of the time you'd want separate staircases for each condition in a factorial, I think?). Your example of duplicate/repmat for adaptive also works (or just needs a few lines to make it work), so with proper documentation users can choose whether to go deep or shallow. But again, the default case would be the most readable.

In other words, the current version allows users to be completely specific (use repmat/duplicate) or use singleton expansion.

I understand that you want to prevent users from specifying something that is different from what they intended, but the trade-off is that the code gets less readable, which also increases the risk of errors. Wouldn't a few examples in the factorial class (and in the demos) be enough to show the different usage? Another option would be to have a nice visual representation of the design that shows which parms are varied and which are the same across conditions. I'll think about that last one; it will be useful regardless of the specification format.

If you feel that singleton expansion is just too dangerous, I could put a flag 'AllowSingletonExpansion' in cic, which a lab could set to false to force fully specified designs.

I agree about "conditions" they are not necessary (only as elements of the factorial). Users should just specify a one-level factorial (which, for simplicity, should still be called 'design', I think).

So here's my proposal:

  1. Keep singleton expansion (with a cic flag to disallow it)
  2. Document shallow/deep copies
  3. Rename factorial to design
  4. Remove single condition specifications from cic
  5. Generate a visual for a design that shows how singletons were expanded

If you agree I'll implement this, update demos, and commit/push to the master branch.

B

On Tue, Nov 29, 2016 at 1:38 AM Shaun Cloherty notifications@github.com wrote:

I think Adam and I have come to the conclusion that the root of the problem the proposed design class would be trying to solve is the ambiguity surrounding singleton expansion in the factorial class.

At present, singletons are expanded silently behind the scenes, i.e.,

myFac.fac1.fix.X = {-10 0 10};

myFac.fac1.fix.Y = 0;

causes fix.Y to be expanded to be a 1x3 vector, equivalent to,

myFac.fac1.fix.X = {-10 0 10};

myFac.fac1.fix.Y = repmat(0,1,3);

That has been ok up until now because it was pretty safe to assume that that is actually what the user intended. However, the new adaptive class(es) change things in that it is no longer clear what should happen wrt expansion of singleton adaptive objects. Should you get multiple independent adaptive objects or should you multiple references (handles) to the same adaptive object.

As it stands today, the adaptive class overrides repmat() to return deep copies (i.e., duplicates) of the singleton object, so

myFac.fac1.fix.X = {-10 0 10}; myFac.fac1.fix.Y = plugins.jitter(c,{0,4},'distribution','normal','bounds',[-5 5]);

produces three independent jitter objects (or three staircases if I'd used a staircase object in the example). That isn't obvious since this behaviour of repmat (called behind the scenes, and provided by the adaptive class) is actually different to that of repmat for all other classes and data types, and specifically, for other handle classes. When you repmat() a handle object, you usually get duplicate handles to a single object, not deep copies (duplicates) of the object.

One solution, which Adam and I have discussed is to not allow singleton expansion behind the scenes in the factorial class. Instead, force the user to explicitly decide, and specify, if they want multiple independent objects (duplicates, or deep copies), or if they want multiple handles to a single object. For example, the scenario above:

myFac.fac1.fix.X = {-10 0 10}; myFac.fac1.fix.Y = plugins.jitter(c,{0,4},'distribution','normal','bounds',[-5 5]);

would cause an error. Forcing the user to specify either:

myFac.fac1.fix.X = {-10 0 10};

myFac.fac1.fix.Y = repmat(plugins.jitter(c,{0,4},'distribution','normal','bounds',[-5 5]).1,3);

producing multiple handles to a single adaptive object (in this case a jitter object), or

myFac.fac1.fix.X = {-10 0 10};

myFac.fac1.fix.Y = duplicate(plugins.jitter(c,{0,4},'distribution','normal','bounds',[-5 5]),1,3);

producing multiple independent adaptive (i.e., jitter) objects.

The following would also cause an error:

myFac.fac1.fix.X = {-10 0 10};

myFac.fac1.fix.Y = 0;

so there is a backwards incompatibility there. Existing scripts with usage like that would need to be changed to:

myFac.fac1.fix.X = {-10 0 10};

myFac.fac1.fix.Y = repmat(0,1,3);

or maybe

myFac.fac1.fix.X = {-10 0 10};

myFac.fac2.fix.Y = 0;

depending on the context.

This would require a couple of fairly small changes:

  1. remove singleton expansion in the factorial class and raise an error instead,
  2. remove the repmat() method in the adaptive class, and
  3. modify the behaviour of the duplicate() method in the parent plugin class to accept a size argument to produce multiple (deep) copies (so it could be called with a syntax something like the example above).

The advantage of this solution is that the behaviour of repmat() and duplicate() would be consistent across all classes and data types.

Now, the other point raised above wrt the proposed design class is the idea of so called "conditions" which one doesn't think of as part of a factorial design. I think we (Adam and I) concluded that actually this is was poorly conceived. If there is a condition that one want's to specify outside an existing factorial design, it is actually really a second factorial object, whose factor(s) have only one level.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/klabhub/neurostim-ptb/pull/63#issuecomment-263440824, or mute the thread https://github.com/notifications/unsubscribe-auth/ALbI0_dQ3tP-2ZVYOyhDy8JaY7PjRGfwks5rC3PygaJpZM4K2Nkd .

adammorrissirrommada commented 7 years ago

My issue is this:

"It is a variable that happens to have the same value across all levels of fac1."

That means it actually just has a constant value for this entire design object! Or, more accurately, it is its own factor that only has only level, which is then crossed with all combinations of other factors. In my mind, it simply has no place being in fac1 at all. And it doesn't save you any time. Instead, your one line would be:

myFac.fac3.fix.Y = 0

which is more readable, I would argue!

So its not only that I think it is ambiguous and dangerous (in the case handles), I think it just doesn't make conceptual sense! But if you like it, I'm happy to have the flag that switches it off.

On the related issue: duplicate() already serves as a means to create a new instance based on an existing one, so adding a size element would make it behave the same as elsewhere in Matlab (e.g. ones, rand, etc.). For the same reason, I still think repmat() should not be overloaded and should be left for duplicating a specific instance/handle.

Maybe we need to have a two- or three-way Skype conversation to thrash this out! (but after my conferences, i.e. after Dec 7)

cnuahs commented 7 years ago

I agree that there is considerable value to be had in readability. Could we achieve the same level of readability without singleton expansion by assigning any variable that has the same value across all levels of a factor to another factor? For example:

myFac.fac1.fix.X = {-10 0 10}; myFac.fac1.fix.Y = 0;

arguably should really be

myFac.fac1.fix.X = {-10 0 10}; myFac.fac2.fix.Y = 0;

that would give the same effect, is just as readable, but there is no need for singleton expansion and hence no ambiguity wrt use of adaptive objects.

bartkrekelberg commented 7 years ago

For these constants, I think this is pretty much cosmetic.

I don;t think there is a logical reason why fac2.Y = 0 is better ("it is a factorial with one level") than fac1.Y = 0 ("this variable is also set to some value in this set of conditions"). It may just be a matter of perspective; I think of the factorial specification as "a bunch of variables that are set before running a specific condition" , which is why fac1.Y=0 does not look so strange to me.

But I agree that because it applies to all conditions withing the design, the more logical place would be myFac.fix.X = 0; as this makes it clear it has nothing to do with the factors (and avoids the odd factor with a single level). I think that was Adam's original suggestion for the new design class. I still like that.Thinking of the new class as a design class rather than a factorial class also helps, I think.

The issues for adaptive parameters are more complicated as there are situations where they should be shared across conditions and some where they should not. I realized that the current version is not ideal as it is not clear what happens when you have two factors

myFac.fac1.fix.X = {-10 0 10}; myFac.fac2.fix.Y = staircase;

Current implementation creates one staircase (Fac2) shared by all three X positions. And, I think that there is currently no way to specify a separate adaptive for each condition in a two-factor design. (Which should be the most common situation).

Following the same logic as above, how about using

myDesign.fac1.fix.X = {-10 0 10}; myDesign.fac2.fix.contrast = 0:0.1:1; myDesign.fix.Y = staircase;

The staircase does not depend on the factors, so it is specified directly on the design, not the fac1,fac2. I'd say this notation should create a separate staircase for each condition (because it is more common). So internally we'd use duplicate to expand the singleton.

To use shared staircases with two factors, the user would have to create a set of adaptive objects (with either repmat or duplicate) and we could create a convention that the first factor is the row dimension of adaptives, second factor column dimension etc. Given that this scenario is rare (I think?) this rather cumbersome specification should be ok?

B

On Tue, Nov 29, 2016 at 10:21 AM Shaun Cloherty notifications@github.com wrote:

I agree that there is considerable value to be had in readability. Could we achieve the same level of readability without singleton expansion by assigning any variable that has the same value across all levels of a factor to another factor? For example:

myFac.fac1.fix.X = {-10 0 10}; myFac.fac1.fix.Y = 0;

arguably should really be

myFac.fac1.fix.X = {-10 0 10}; myFac.fac2.fix.Y = 0;

that would give the same effect, is just as readable, but there is no need for singleton expansion and hence no ambiguity wrt use of adaptive objects.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/klabhub/neurostim-ptb/pull/63#issuecomment-263517864, or mute the thread https://github.com/notifications/unsubscribe-auth/ALbI03esS7_0qrj8iI2uEI2D584yUra9ks5rC-59gaJpZM4K2Nkd .

adammorrissirrommada commented 7 years ago

For me:

myDesign.fac1.fix.X = {-10 0 10}; myDesign.fac2.fix.contrast = 0:0.1:1; myDesign.fix.Y = staircase;

should use the same staircase instance for all conditions. The meaning of this kind of syntax is “this property is constant across conditions”. If it’s a jitter object or a staircase, it’s still constant in the sense that it is always equal to the value of that same instance.

I actually don’t think the “unique staircase for each condition of the factorial matrix” is the more common design. I think that would be rare. More likely would be this kind situation:

myDesign.fac1.fix.X = {-10 0 10}; myDesign.fac1.dots.coherence = {staircase1, staircase2, staircase3}; myDesign.fac2.fix.contrast = 0:0.1:1;

which uses a different staircase for each level of one factor (but the same staircase for all levels of other factors).


Zooming out a bit, I think this actually highlights a broader issue and a possibly a broader solution. I think it’s fair to say that where we are disagreeing is whether things should be explicitly or implicitly specified, i.e. whether there should be some interpretation of what people mean when they set something. I prefer explicit (i.e. don’t assume they mean create new instances) – make them say it.

Consider a related issue of how to specify designs that include conditions that are not completely crossed in the factorial design. E.g. suppose I have a four-factor experiment, except one condition (that is, one particular cell [i,j,k,l] in that factorial matrix) should not exist, or, it should include the setting of additional or different property values – they aren’t constant across the whole experiment, and they aren’t completely crossed either. For example, I might want a stimulus to appear at a different time in one particular condition. It would be a nightmare for a user to have to consider this a one factor experiment and then enumerate all the combinations themselves.

This could be solved, as could the staircase issues we’ve been discussing, if there was a convenient way to modify or over-rule an existing factorial specification through an approach that includes subscripting to refer to particular conditions (where factor 1 is i, fac 2 is j, fac 3 is k etc.). This could include assigning staircase instances to particular conditions as desired, or even to over-ride the property values that would otherwise be set by the factorial specification.

Anyway, what is clear is that we need to talk on Skype!

I'm back from conferences, so I can chat over the weekend if you like

bartkrekelberg commented 7 years ago

Summary of discussion with Adam today:

  1. We'll create a new design class based on factorial
  2. The .fac1 .fac2 notation/specification will be used to generate a fully crossed matrix of conditions.
  3. We'll add a new way to modify some of the elements in that matrix of conditions: d.conditions(1,2).fix.X = 5, for instance to change a single entry in the matrix (1st level fac1, 2nd level fac2) 3b. This can also be used to setup a design that isn't a factorial (e.g. a single condition , or some unrelated conditions) d.conditions(1).fix.X = 0 ; d.conditions.(2).gabor.Y = 5; We'll need to have some check that conditions specified this way are contiguous (i.e. no conditions without specifications).
  4. Adaptive parameters are also set via the d.conditions(i,j) interface. (because they are not part of any factorial).
  5. Singleton expansion of adaptive parameters will assign the same handle (i.e. shared staircases are defaults). d.conditions(:,:).fix.X = jitter() assigns a single jitter object to all conditions in the 2-way factorial, d.conditions(:,1).fix.X = jitter() applies jittering only to the first level of the second factor.
  6. We need a way to show/summarize the design graphically
  7. There is an issue with setting parameters in one condition, but not in others. The plugin will keep that value once it is set. We will create a function that checks that a variable that is set in one condition, is set in all conditions. (Or a function that re-applies the default in such cases).

Once I've implemente these changes I'll merge the adaptive branch back to the master.

bartkrekelberg commented 7 years ago

As promised... issues 1..7 discussed above are fixed in this merge, now in the master.