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

Thread local state for caffe instead of singleton #2367

Closed
wants to merge 1 commit into from

Conversation

cypof
Copy link
Member

@cypof cypof commented Apr 25, 2015

Part of #2351. Duplicate of #2067, it adds some initialization code in internal_thread, to make sure that when a thread is created, the parent thread state for Caffe is inherited, e.g. mode (GPU/CPU) and device. The random number generator is also initialized using a sample from the parent thread, which makes sure each thread will have a different sequence, but while keeping the whole app deterministic.

@flx42
Copy link
Contributor

flx42 commented Apr 28, 2015

I feel you have 3 different changes here:

  1. Make Caffe context TLS.
  2. Add seed/mode/device to each thread context.
  3. Change the way threads are stopped.

Is there a reason to have those 3 features in one PR?
Thanks!

@cypof
Copy link
Member Author

cypof commented Apr 28, 2015

No reason, I can split them. But I feel they should be merged together as a constraint. To make sure a thread cannot be created without its local state initialized.

@flx42
Copy link
Contributor

flx42 commented Apr 28, 2015

I see, but how bad is it if a thread is created without initializing a local state?
For instance, right now I mostly care about classification, I'm not sure the constraint is relevant in this case.

For 1) + 2), I understand your point, let's see what the maintainers have to say.
But 3) is not related to this state constraint your mentioned, right?

@cypof
Copy link
Member Author

cypof commented Apr 28, 2015

Forgetting to initialize the random seed will most likely just make runs not deterministic, which might be OK. Less likely is to have parts of an app to run on the wrong device. It doesn't cost anything to force inheriting state from the parent. I will split 3).

@cypof
Copy link
Member Author

cypof commented Apr 28, 2015

Well it costs the creation of an instance a Caffe instance, which means handles to CUDA libraries in GPU mode. But it is very likely to be created later during execution of the thread.

@shelhamer
Copy link
Member

Closing since everything is bundled and updated in #2114 (and now #2870).

@shelhamer shelhamer closed this Aug 6, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants