Improve random number generator helper in the tree module #3397

Merged
merged 1 commit into from Jul 16, 2014

Conversation

Projects
None yet
5 participants
@arjoly
Owner

arjoly commented Jul 16, 2014

Taken from #3173

Ping @vene, @jnothman, @amueller

@amueller

This comment has been minimized.

Show comment Hide comment
@amueller

amueller Jul 16, 2014

Owner

LGTM. Thanks for pulling this out :D

Owner

amueller commented Jul 16, 2014

LGTM. Thanks for pulling this out :D

sklearn/tree/_tree.pyx
"""Generate a random double in [0; 1)."""
- return <double> our_rand_r(random_state) / <double> RAND_R_MAX
+ return (low + (high - low) * <double> our_rand_r(random_state)
+ / <double> RAND_R_MAX)

This comment has been minimized.

Show comment Hide comment
@jnothman

jnothman Jul 16, 2014

Owner

This indentation is a bit nonstandard. Otherwise, LGTM.

@jnothman

jnothman Jul 16, 2014

Owner

This indentation is a bit nonstandard. Otherwise, LGTM.

@arjoly

This comment has been minimized.

Show comment Hide comment
@arjoly

arjoly Jul 16, 2014

Owner

Fix style :-)

Owner

arjoly commented Jul 16, 2014

Fix style :-)

@ogrisel

This comment has been minimized.

Show comment Hide comment
@ogrisel

ogrisel Jul 16, 2014

Owner

Looks good to me as well. Merging. @MechCoder, please reflect this change in #3335.

Owner

ogrisel commented Jul 16, 2014

Looks good to me as well. Merging. @MechCoder, please reflect this change in #3335.

ogrisel added a commit that referenced this pull request Jul 16, 2014

Merge pull request #3397 from arjoly/tree-factor-rand
Improve random number generator helper in the tree module

@ogrisel ogrisel merged commit 9c8f656 into scikit-learn:master Jul 16, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
@MechCoder

This comment has been minimized.

Show comment Hide comment
@MechCoder

MechCoder Jul 16, 2014

Owner

Thanks. But do I need to change it? low is zero in this case :)

Owner

MechCoder commented Jul 16, 2014

Thanks. But do I need to change it? low is zero in this case :)

@arjoly arjoly deleted the arjoly:tree-factor-rand branch Jul 16, 2014

@jnothman

This comment has been minimized.

Show comment Hide comment
@jnothman

jnothman Jul 16, 2014

Owner

I don't understand your question, @MechCoder. The function is (hopefully) inlined, so low being 0 will be compiled out. But having a consistent interface that refactors code is nice.

Owner

jnothman commented Jul 16, 2014

I don't understand your question, @MechCoder. The function is (hopefully) inlined, so low being 0 will be compiled out. But having a consistent interface that refactors code is nice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment