Update Docs: Pooling #199
Update Docs: Pooling #199
Conversation
pberkes
left a comment
pberkes
left a comment
There was a problem hiding this comment.
Thank you, I added a could of minor comments.
Please also remove the file user_guides/Pooling-Utility , and its reference in _toc.yml. It refers to a work-in-progress page.
docs/examples/pooling_utilities.md
Outdated
|
|
||
| ```{code-cell} ipython3 | ||
| #act = pool_blocks(inp, max_tol=0.4, max_gap=10, max_length=10) |
There was a problem hiding this comment.
This line is commented out, is it meant to show how the utility is called? Maybe introducing it with "the general form of the manual pooling utility looks like this:" might make sense
docs/examples/pooling_utilities.md
Outdated
|
|
||
| This brakes the large blocks up again allowing us to notice if there was more variability over time than expected. | ||
|
|
||
| The last option gives us a different rule to achieve something in a similar direction: We can enforce that a block must be collected en bloc like this: |
There was a problem hiding this comment.
I'm not sure I got this: a block can be of unlimited size, and there is a maximum gap of 0 between blocks? Is it not like the first case without options?
There was a problem hiding this comment.
The first one pools all trials within a tolerance. The next one pools only up to a block size of 25. The last one pools only subsequent trials. If I understand correctly.
There was a problem hiding this comment.
got it, thanks. maybe "we can enforce to pool only subsequent trials", instead of "must be collected en block"?
| # We added a factor of 1000 to make visually similar to the MATLAB version | ||
| size = np.sqrt(_data_size / data[:, 2])*(data_size*1000) | ||
| # We added a factor of 100 to make visually similar to the MATLAB version | ||
| size = np.sqrt(data[:, 2])*(data_size*100) |
There was a problem hiding this comment.
As far as I know, the factor of 100-1000 was always there, and recently @guillermoaguilar added a data_size option in case this default is not good enough. Maybe we should move the factor directly in the keyword argument? I.e. make data_size = 100 by default, and then the user can adjust it if necessary
There was a problem hiding this comment.
oh but I see now the "reversing" of the logic... it's true, it seems the wrong way around
There was a problem hiding this comment.
data_size and _data_size were two different values though. That was causing the problem.
There was a problem hiding this comment.
true, sorry I missed that
There was a problem hiding this comment.
its true that we could abandon the 100 scaling factor... on the other hand its also intuitive to have the input value 1 be a reasonable size already. I dont mind either way.
There was a problem hiding this comment.
no need for changes, afaic
|
LGTM |
|
Ok, fixed the phrasing on the one remaining point. Good to merge from my side! |
|
The changes look good. The log-space notebook now has a plot that looks like below for some reason... would mind fixing it please? then it can be merge, as far as I'm concerned 
|
|
LGTM |
2 participants
I fixed the page for the Pooling utility in the docs.
There was a strange bug (?) in the
plot_psychometric_functioncode which reversed the logic of smaller dots representing blocks with fewer stimulus levels. Its strange because it seemed very intentional. I changed it here, but if you can see any reason not to please let me know. That function was recently touched by @guillermoaguilar and @pberkes but the line seems to have been there since always.Adresses issue #83