placed context stack inside thread-local data space

[burnpanck]

Currently, PyMC3's model-context suggests that the effect is scoped to the body of the context manager. However, this is not the case: Since it changes global state (the class attribute contexts is shared globally within the interpreter), it affects any other PyMC3 code that could be running in a different thread. By making the context stack thread-local, the effect of the changes are properly confined to the code inside the context manager.

Fixes #1552

[twiecki]

Traceback (most recent call last):
  File "/home/travis/minconda3/envs/testenv/lib/python3.5/threading.py", line 914, in _bootstrap_inner
    self.run()
  File "/home/travis/minconda3/envs/testenv/lib/python3.5/threading.py", line 862, in run
    self._target(*self._args, **self._kwargs)
  File "/home/travis/minconda3/envs/testenv/lib/python3.5/multiprocessing/pool.py", line 429, in _handle_results
    task = get()
  File "/home/travis/minconda3/envs/testenv/lib/python3.5/multiprocessing/connection.py", line 251, in recv
    return ForkingPickler.loads(buf.getbuffer())
  File "/home/travis/build/pymc-devs/pymc3/pymc3/distributions/distribution.py", line 18, in __new__
    model = Model.get_context()
  File "/home/travis/build/pymc-devs/pymc3/pymc3/model.py", line 120, in get_context
    return cls.get_contexts()[-1]
  File "/home/travis/build/pymc-devs/pymc3/pymc3/model.py", line 114, in get_contexts
    return cls.contexts.stack
AttributeError: '_thread._local' object has no attribute 'stack'

[burnpanck]

Oh. I'll look into it.

[burnpanck]

Indeed, there was a stupid mistake in my implementation, and I should have tested it before uploading. Now I added a regression test and it does work. However, on my local machine, test_diagnostics (and some other tests?) still fails. Running under py.test (what do you use on travis?), exceptions in threads other than the main thread seem to block indefinitely, which makes it hard to see what's going on. From what I could gather so far, it looks like that test fails somewhere in a multi-processing sampling function. Apparently, some data is serialised via pickle to communicate across threads/processes, which eventually seems to fail possibly due to #895 (by indident, my report there before had nothing to do with this): With this fix here, the receiver doesn't own a model-context any more.

[ColCarroll]

Can't help much in the next few days, but tests are run using nose on travis -- you can launch them by hand with something like

THEANO_FLAGS='gcc.cxxflags="-march=core2"' nosetests -vv tests/this_is_optional.py

This is equivalent to

./scripts/test.sh -vv tests/this_is_optional.py

If you have Docker running on your machine, you could also ./scripts/start_container.sh to start a pymc3 container, then run the tests inside there (more details are in the CONTRIBUTING.md).

I'm not sure why, but most scientific python projects use nose instead of pytest.

[springcoil]

I'd say that it's a historical legacy using nose rather than pytest. If this project were starting now we'd probably use pytest.

…

On Mon, Nov 28, 2016 at 4:20 PM, Colin ***@***.***> wrote: Can't help much in the next few days, but tests are run using nose on travis -- you can launch them by hand with something like THEANO_FLAGS='gcc.cxxflags="-march=core2"' nosetests -vv tests/this_is_optional.py This is equivalent to ./scripts/test.sh -vv tests/this_is_optional.py If you have Docker running on your machine, you could also ./scripts/start_container.sh to start a pymc3 container, then run the tests inside there (more details are in the CONTRIBUTING.md). I'm not sure why, but most scientific python projects use nose instead of pytest. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1555 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA8DiGF9BRFFhOgoDSgakjFnBjb_jht5ks5rCv8ygaJpZM4K85ij> .

-- Peadar Coyle Skype: springcoilarch www.twitter.com/springcoil peadarcoyle.wordpress.com

[burnpanck]

I merged my PR #1560 into this. According to Travis, solving the pickle problem indeed fixes this PR here too.

[twiecki]

Probably want to rebase.

[twiecki]

LGTM. @ColCarroll think we should merge this into master or 3.1?

[ColCarroll]

Seems like a bugfix, not a feature, so I'd go with master.

[twiecki]

Thanks @burnpanck!