Fix calibrations #177
Fix calibrations #177
Conversation
Setting dev up-to-date with master, plus incorporating additional changes made directly to dev (that were not made to master)
…aster 'signalsy' approach
reverted premature commit
added future training flag to AlyxPanel
…ent log formatting
Added bare-bones index file
* Started setVals test for param editor * Finished ParamEditor tests; changed Timeline flags to switches
|
Seems the tests are failing catastrophically. How do they run on your rig? |
| % Calls PTB's `DrawFormattedText` to display text to the PTB Window | ||
| % | ||
| % Inputs: | ||
| % `text`: the text to be displaye |
| % `nx`: | ||
| % `ny`: | ||
| % | ||
| % Examples: |
There was a problem hiding this comment.
Please remove this or add a TODO (or add examples)
There was a problem hiding this comment.
There was a problem hiding this comment.
(Documenting existing code is never a waste of time! :)
| % `wrapAt`: a number for the max number of characters in a line of text | ||
| % | ||
| % Outputs: | ||
| % `nx`: |
There was a problem hiding this comment.
May as well fill this in. It's in the PTB docs
There was a problem hiding this comment.
| @@ -197,9 +197,10 @@ function open(obj) | |||
| end | |||
| Screen('Preference', 'SuppressAllWarnings', true); | |||
| % setup screen window | |||
| %obj.PxDepth = 32; | |||
There was a problem hiding this comment.
Instead of commenting things out with no explanation, leave some sort of description or remove entirely. Otherwise it's just clutter that will mean nothing to no one in a few month's time.
MATLAB doesn't save parameters whose defaults are unchanged. This means that you might end up breaking someone's rig by making this change. If you want to go ahead with it let everyone know on slack to check this works on their rigs before merging into master.
There was a problem hiding this comment.
this was a little egg for you : ) want to see if there's any reason it's set to 32?
Regarding the not saving of this property, I tested it out on the training rigs and BII and all was good, so i think should be fine, but I can send a message to the slack as well
There was a problem hiding this comment.
I don't know enough about graphics tbh but there's often a good reason for things to be set as they are. It may be that controlling the bit depth is essential for generating the visual stimuli correctly. If you let the system determine the value I suppose it will vary across rigs which we don't necessarily want. I know that bit depth is important in rendering low contrasts and that there are some assumptions made in parts of the code (e.g. https://github.com/cortex-lab/signals/blob/9f4f5d068805d8dc715e332449086e42fa396da2/%2Bvis/gaussianLayer.m#L18-L22). From what I've read 32 is an odd value for bit depth, and it may simply be a carry-over from when we used a particular monitor.
Long story short, I've come to realize that one shouldn't flippantly change bits of code when you don't understand them. You should ask someone like Krumin about this and make sure everyone tests their recording rigs or at least approves this change.
There was a problem hiding this comment.
yup fair point, I'll do this
|
Haven't run the current tests on this yet because I wanted your general opinion on this PR. I can check tomorrow. And yeah, I'll add test(s) here too. |
Fixes for #4, #128, #164
If this type of fix looks good, let me know. I also want to improve the doc for hw.ptb.Window, make function/method names clearer, and create simple config scripts explaining how to run gamma calibration and reward valve calibration (these may share some redundancy with what goes on ReadTheDocs, but I think that's fine).
If sounds good, I'll do all of the above in this branch.