MRG: Tracker dealer example #310

maddycapp27 · 2017-06-07T14:57:20Z

This is just a simple example for the TrackerDealer. It's pretty similar to the tracker staircase example but please let me know if there are any changes that should be made. Thanks much!

rkmaddox

Mind posting what the plot looks like after it runs?

rkmaddox · 2017-06-07T14:58:28Z

examples/stimuli/tracker_dealer.py

+:class:'expyfun.stimuli.TrackerDealer'
+
+In this case, a modeled human subject generates two curves (one for each trial
+type: 1 & w)


rkmaddox · 2017-06-07T15:01:36Z

examples/stimuli/tracker_dealer.py

+rng = np.random.RandomState(1)
+
+while not tr.stopped:
+


Don't need a blank line after while, if, etc.

rkmaddox · 2017-06-07T15:01:48Z

examples/stimuli/tracker_dealer.py

+
+# Plot the results
+for i in [0, 1]:
+


Same, no blank line.

maddycapp27 · 2017-06-07T15:05:03Z

Sure thing, here are the plots:

rkmaddox · 2017-06-07T15:08:27Z

Cool, looks like it's working. They're probably better as subplots so only one plot is made, and that can serve as the thumbnail for the documentation.

larsoner · 2017-06-07T15:12:00Z

I have a real-life use case I need to make -- maybe this can be adapted so I can be lazy and copy-paste more easily.

KC wants interleaved adaptive tracking, one from above and one from below. This is a thing, right @rkmaddox? And TrackerDealer is meant to do it? Can this example be made to do it if it doesn't already? (If it does, update docs to say so please.)

rkmaddox · 2017-06-07T15:14:36Z

Yes, it could easily do it. You just make two trackers in TrackerDealer with different start points.

However, while a fine application, it's not what it was made to do. The reason for making TrackerDealer was so that you could have interleaved identical trackers for different experimental conditions, not different trackers for the same condition.

So I think the example should stay as is, but it should be super easy for you to do what you want. I am more than happy to help with that if it isn't clear from the docs and example how to do it.

EDIT: it is worth pointing out the only reason this can do it is because of the changes you forced me to make to the way TrackerDealer is instantiated, so thanks for that.

larsoner · 2017-06-07T15:14:48Z

FYI @maddycapp27 a better way to get at what @rkmaddox asked about is to look at CircleCI, which builds all examples. You can see the output if you click the link (sign in with GitHub on CircleCI) and then follow the "Artifacts" tab to the HTML output like here:

https://210-11614571-gh.circle-artifacts.com/0/home/ubuntu/expyfun/doc/_build/html/auto_examples/stimuli/tracker_dealer.html

larsoner · 2017-06-07T15:15:20Z

I am more than happy to help with that if it isn't clear from the docs and example how to do it.

Yes please make a PR to add docs and ideally an example if you or @maddycapp27 have time

rkmaddox · 2017-06-07T15:17:52Z

On the link @Eric89GXL sent, it looks like ":class:’expyfun.stimuli.TrackerDealer’" isn't rendering properly.

rkmaddox · 2017-06-07T15:19:19Z

Yes please make a PR to add docs and ideally an example if you or @maddycapp27 have time

There are docs for TrackerDealer. Do they need to modified to make it clear that this is possible?

maddycapp27 · 2017-06-07T15:19:22Z

The render is my bad--used the wrong `

larsoner · 2017-06-07T15:21:28Z

Do they need to modified to make it clear that this is possible?

I was concerned that your statement about it not being designed to handle interleaved top-down / bottom-up (or whatever it's called) meant there had to be some special treatment. If no docs are necessary, it would be good from a pedagogical standpoint to have a simple example, at least so that folks like me can get the terminology right :)

rkmaddox · 2017-06-07T15:22:37Z

Yeah, happy to make an example. It will be a very small modification of @maddycapp27's. The docs don't have any wording about how it should be used, so I think your use case is just as legit as what it was designed for. But a separate example is the best way to show that I think.

rkmaddox · 2017-06-07T15:23:29Z

When this PR gets merged, I'll ask @maddycapp27 to make the modifications and submit another PR based on that successful one.

larsoner · 2017-06-07T15:24:13Z

When this PR gets merged, I'll ask @maddycapp27 to make the modifications and submit another PR based on that successful one.

Awesome, thanks

rkmaddox · 2017-06-07T15:25:30Z

Glad to see that TrackerDealer is even more useful than it was intended to be :-)

larsoner

Minor tweaks, otherwise LGTM

larsoner · 2017-06-07T15:27:39Z

examples/stimuli/tracker_dealer.py

+                                           lower=chance, slope=slope))
+
+# Plot the results
+axes = [plt.subplot(211), plt.subplot(212)]


axes = plt.subplots(2, 1)[1]

larsoner · 2017-06-07T15:27:53Z

examples/stimuli/tracker_dealer.py

+    ax.legend(loc='best')
+    ax.set_title('Adaptive track of model human trial type {} (true threshold '
+                 'is {})'.format(i + 1, true_thresh[i]))
+    plt.tight_layout()


avoid global plt commands, prefer fig.tight_layout() here

larsoner · 2017-06-07T15:28:26Z

examples/stimuli/tracker_dealer.py

+from expyfun.analyze import sigmoid
+import matplotlib.pyplot as plt
+
+# define parameters of modeled subject (using sigmoid probability)


FYI if you want you can make pretty sections in the output like this:

https://github.com/mne-tools/mne-python/blob/master/tutorials/plot_artifacts_correction_filtering.py#L49

that render like this:

https://mne-tools.github.io/dev/auto_tutorials/plot_artifacts_correction_filtering.html#removing-power-line-noise-with-notch-filtering

might be overkill in this example but it's something to consider when writing pedagogical examples

Thanks, I'll definitely keep that in mind for the future! For this example, I agree that it's not very necessary though.

rkmaddox · 2017-06-07T15:32:34Z

examples/stimuli/tracker_dealer.py

+
+# parameters for the tracker dealer
+max_lag = 2
+rand = None


I think @Eric89GXL usually likes this to be called rng. And I think in this case it is better to seed it, so rng = np.random.RandomState(0). Mess around with the seed value until you get plots that end up with thresholds within 1 or 2 of their true value. Seeding the RNGs will make the plots the same every time, and having the computed thresholds close to the the true ones in the example will be less confusing for people.

Ahhh, ok, I was confused by that in the other example. Will fix!

Oops, probably shouldn't have been None there, either.

maddycapp27 · 2017-06-07T15:48:36Z

examples/stimuli/tracker_dealer.py

@@ -52,7 +52,7 @@ def callback(event_type, value=None, timestamp=None):
                   change_criteria, change_rule, x_min, x_max) for i in [0, 1]]

 # initialize TrackerDealer object
-tr = TrackerDealer(tr_UD, max_lag, rand)
+tr = TrackerDealer(tr_UD, max_lag, rng)

 # Initialize human state
 rng = np.random.RandomState(1)


Is it stylistically okay that I overwrite rng here? I could see it being confusing to someone just skimming the example.

Better to do rng_human and rng_dealer or something like that.

larsoner · 2017-06-07T16:14:59Z

@rkmaddox feel free to merge when you and the CIs are happy

larsoner · 2017-06-07T17:33:06Z

@drammock feel free to look, too, if you want

drammock · 2017-06-07T17:41:33Z

examples/stimuli/tracker_dealer.py

+up = 1
+down = 1
+step_size_up = [9, 3]
+step_size_down = [3, 1]


just so I'm clear... the first element of each of these is "initial step size" and the second element kicks in after change_criteria[1] reversals have occured?

Correct--do you think there should be a comment to that effect?

+1 for comment

yeah, a comment would be good (since initially I thought they were different values that were going to be fed into the two different TrackerUDs that were getting created).

drammock · 2017-06-07T17:54:16Z

@Eric89GXL the appveyor error seems unrelated... I went in and tried to re-start that job but either I wasn't empowered to, or just couldn't figure out how.

drammock · 2017-06-07T18:00:03Z

one last comment: when I pull down this branch, the permissions on tracker_dealer.py don't match the rest of the examples (it is marked as executable). @Eric89GXL @rkmaddox do we care?

larsoner · 2017-06-07T18:01:06Z

do we care?

I'm all for uniformity so if someone gets around to changing it, go ahead. But I wouldn't hold up the PR for it, as I don't think it should have much consequence.

maddycapp27 · 2017-06-07T18:02:26Z

Is the permissions thing something I did?

drammock · 2017-06-07T18:02:27Z

cool. if another example is going to get generated for @Eric89GXL's use case, maybe we can change the permissions on tracker_dealer.py at that point.

drammock · 2017-06-07T18:07:14Z

@maddycapp27 yes (though perhaps not intentionally). The executable bit is in fact the only aspect of file permissions that git keeps track of (it doesn't track user/group permissions).

maddycapp27 · 2017-06-07T18:11:08Z

Certainly not intentionally (I'm a bit ignorant about such things). I'm not sure how to fix that.

larsoner · 2017-06-07T18:16:25Z

(I'm a bit ignorant about such things). I'm not sure how to fix that.

Something like chmod a-x tracker_dealer.py :)

And I'm sure there are some around here who will appreciate your (un?)intentional "bit ignorant" pun... (for those that don't, we're talking about setting the executable bit)

maddycapp27 · 2017-06-07T18:21:10Z

@Eric89GXL Thank you!

Alas not an intentional pun~ I appreciate the explanation :)

larsoner · 2017-06-07T19:01:43Z

Thanks @maddycapp27, looking forward to the next example :)

maddycapp27 added 5 commits May 16, 2017 08:17

typo in tracker

9182025

add example

ac020ba

parameter fix convergence

baf8e0b

file in correct place

a40640c

tracker staircase odd thing

bdb0e28

rkmaddox reviewed Jun 7, 2017

View reviewed changes

small errors

a116cee

fix ticks and subplot

cee996f

larsoner reviewed Jun 7, 2017

View reviewed changes

rkmaddox reviewed Jun 7, 2017

View reviewed changes

random seed

8867210

maddycapp27 commented Jun 7, 2017

View reviewed changes

maddycapp27 added 2 commits June 7, 2017 11:58

function def need more whitespace

e98a7f2

disambiguate rng

47f029f

larsoner approved these changes Jun 7, 2017

View reviewed changes

drammock reviewed Jun 7, 2017

View reviewed changes

more explainy

73cf303

typo and permissions

d1e7d6d

larsoner merged commit 793e387 into LABSN:master Jun 7, 2017

maddycapp27 deleted the tracker_dealer_example branch June 7, 2017 19:25

MRG: Tracker dealer example #310

MRG: Tracker dealer example #310

Conversation

maddycapp27 commented Jun 7, 2017

rkmaddox left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maddycapp27 commented Jun 7, 2017

rkmaddox commented Jun 7, 2017

larsoner commented Jun 7, 2017

rkmaddox commented Jun 7, 2017 • edited

larsoner commented Jun 7, 2017

larsoner commented Jun 7, 2017

rkmaddox commented Jun 7, 2017

rkmaddox commented Jun 7, 2017

maddycapp27 commented Jun 7, 2017

larsoner commented Jun 7, 2017

rkmaddox commented Jun 7, 2017

rkmaddox commented Jun 7, 2017

larsoner commented Jun 7, 2017

rkmaddox commented Jun 7, 2017

larsoner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

larsoner commented Jun 7, 2017

larsoner commented Jun 7, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

drammock commented Jun 7, 2017

drammock commented Jun 7, 2017

larsoner commented Jun 7, 2017

maddycapp27 commented Jun 7, 2017

drammock commented Jun 7, 2017

drammock commented Jun 7, 2017

maddycapp27 commented Jun 7, 2017

larsoner commented Jun 7, 2017

maddycapp27 commented Jun 7, 2017

larsoner commented Jun 7, 2017

rkmaddox commented Jun 7, 2017 •

edited