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

Is it possible to change the array length of parameters in eui.ParamEditor? #311

Open
dbirman opened this issue Dec 11, 2020 · 3 comments
Open

Comments

@dbirman
Copy link

dbirman commented Dec 11, 2020

Describe the bug
It appears to not be possible to change the length of a parameter in the GUI.

To Reproduce
Define a parameter in a signals experiment either as a single value or an array:

p.example1 = 1;
p.example3 = [1, 2, 3]';

Using the mc GUI or eui.SignalsTest try to change the length of either parameter.

Setting p.example1 to [1,2] will cause this error:

Unable to perform assignment because the size of the left side is 1-by-1 and the size of the right side is 2-by-1.

Error in eui.ParamEditor/update (line 231)
           obj.Parameters.Struct.(name)(:,row) = newValue;

Setting p.example3 to [1,2] will cause the same error but with a different assignment size.

This appears to be happening because of the (:,row) code at line 231, which forces the matrix to stay the same shape. Removing that extra code allows parameters to change length, but it's unclear if that might be breaking something else up/down stream from here.

p.s. (:,row) is flipped, that's a column?

@k1o0
Copy link
Contributor

k1o0 commented Dec 14, 2020

Hi, for better or worse this is the expected behaviour. The issue is that we must enforce an equal number of trial conditions. Trial conditions are defined as parameters with more than 1 column (following MATLAB's column-major order), but the conditions are presented visually as rows in the ParamEditor table, which is kind of confusing, hence the (:, row) is index referring to the table row.

Because the input fields store everything as text, we have to cast the user input back into the underlying format (e.g. '1,2,3' -> [1 2 3]). For most things this is more convenient for the user because they don't have to add braces and quotes, etc. The fact that all trial conditions must share a dimension, and that there is interpretation ambiguity, makes it difficult to implement size and type changes for example when a user writes '1,2,3' do they want three different trial conditions or to change the size of the array?

For most of the experiments we do the parameters are not expected to change type or size. Although this is possible, particularly in Signals, it makes things more difficult because you have to write in cases to deal with dimension mismatches and such. It's very uncommon that a user wants to change the size of their parameters so the safest thing is to throw an error.

We did partially implement a way around this, which was to allow parameters that are cell arrays to be 'free fields', i,e. they could be mixed type and could change type and shape. Again this is tricky because it's hard to interpret the intended input of the user, so with this feature the values all end up a char arrays (if you type '5' do you want it to cast to numeric or keep as a char?).

I guess we could do two things to improve this:

  1. Expose the ParamEditor prop in the mc object, so that users can do these advanced things in the command prompt:
MC = mc;  % start mc
MC.Parameters.Struct.example1 = [1, 2, 3]'  % change parameter from number to array
MC.ParamEditor.buildUI(MC.Parameters)  % clear and rebuild
  1. Add an option in the context menu to change the parameter type. A little window like the one for 'set values' can contain a text box whose input is run through eval, which will parse the input like command prompt input.

  2. We could completely overhaul the Parameters class so that users define two separate structures, one for trial conditions and one for global parameters (Improving definition of global and conditional parameters #144). We could add support for an additional types table that defines the supported types for each parameter.

The first one is obviously very easy to implement, the second one shouldn't take to long but I'd need to add tests for mc which will take a while. The third option would be a lot of work and, aside from avoiding the row/column size confusion, would not be used very often by users. What do you think about these solutions?

@dbirman
Copy link
Author

dbirman commented Dec 14, 2020

Thanks for the detailed explanation. Option 1 seems best, if someone ever implements 3 then the exposed property could be removed.

I might still be misunderstanding, but isn't there also an option here to parse anything that includes brackets as numeric (e.g. [1,2,3]') so that you can then interpret whether it's a row or column vector to tell what the user wants to happen? So [1,2,3] is interpreted as conditional and split into the trial conditions, while [1,2,3]' changes the array size of a global parameter?

@k1o0
Copy link
Contributor

k1o0 commented Jan 4, 2021

I might still be misunderstanding, but isn't there also an option here to parse anything that includes brackets as numeric (e.g. [1,2,3]') so that you can then interpret whether it's a row or column vector to tell what the user wants to happen? So [1,2,3] is interpreted as conditional and split into the trial conditions, while [1,2,3]' changes the array size of a global parameter?

It could have been implemented that way. It has its advantages and disadvantages. For example fringe cases where the underlying type is not numerical there would be problems. The advantage of the current way is that the user doesn't have to worry about array syntax or shape.

I'll come up with a solution for the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants