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

Provide cross-platform seed generation #3306

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Provide cross-platform seed generation #3306

wants to merge 1 commit into from

Conversation

eelstork
Copy link
Contributor

@eelstork eelstork commented Nov 9, 2015

Previously (#842), the use of an entropy source was added so that caffe's RNG uses a random seed by default. Although @netheril96 suggested that we avoid boost::random_device, I contend that incurring a dependency on boost random is a small price to pay for cross-platform compatibility.

Additionally, is there a practical reason to preserve the fallback solution via getpid ?

@eelstork
Copy link
Contributor Author

We shouldn't use std::random_device.

In C++11 std::random_device may be implemented in terms of an implementation-defined pseudo-random number engine if a non-deterministic source (e.g. a hardware device) is not available to the implementation. In this case each std::random_device object may generate the same number sequence.

In constrast, Boost specifies that for those environments where a non-deterministic random number generator is not available, class random_device must not be implemented.

@bhack
Copy link
Contributor

bhack commented Nov 10, 2015

@eelstork If you are interested I've removed all boost dependencies other than for python some months ago at #2537

@eelstork
Copy link
Contributor Author

@bhack although I am interested (and was aware of #2537), I feel that this (removing boost) may be premature. Additionally, the problem at hand (seed generation) inspires caution: C++ 11 isn't always an ideal replacement for Boost.
What is the main reason to remove Boost? Smaller footprint on handhelds?

@bhack
Copy link
Contributor

bhack commented Nov 10, 2015

The effort started to minimize the dependencies for mobile and embedded (specially for forward pass only of trained nets). I.e. in android boost it is not so much a first grade citizen. See #2619

@bhack
Copy link
Contributor

bhack commented Nov 15, 2015

I repeat again. Are we sure that we are not embracing a rearguard position?

@eelstork
Copy link
Contributor Author

@bhack currently caffe builds default to C++98. As mentioned before I'm looking into a workaround so that we can be more inclusive.
@shelhamer can you have a look at this?

@bhack
Copy link
Contributor

bhack commented Nov 16, 2015

@eelstork I know. But after 6 months only Nvidia guys commented to the introduction of c++11 and Boost removal in c++. Also the big #2610 rely on c++11. As pointed also Tensorflow already use c++11 features. The problem is ignoring instances and not what kind of direction maintainers want to take.

@eelstork
Copy link
Contributor Author

Ready for review/merge.

@eelstork
Copy link
Contributor Author

@ronghanghu can you guys have a look at this?

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