{"action":"created","comment":{"url":"https://api.github.com/repos/rust-lang-nursery/rand/pulls/comments/194236712","pull_request_review_id":127376264,"id":194236712,"node_id":"MDI0OlB1bGxSZXF1ZXN0UmV2aWV3Q29tbWVudDE5NDIzNjcxMg==","diff_hunk":"@@ -534,59 +533,100 @@ macro_rules! uniform_float_impl {\n \n             fn new(low: Self::X, high: Self::X) -> Self {\n                 assert!(low < high, \"Uniform::new called with `low >= high`\");\n-                let scale = high - low;\n-                let offset = low - scale;\n-                UniformFloat {\n-                    scale: scale,\n-                    offset: offset,\n+                assert!(low.is_finite() && high.is_finite(), \"Uniform::new called with non-finite boundaries\");\n+\n+                let max_rand = (::core::$uty::MAX >> $bits_to_discard)\n+                               .into_float_with_exponent(0) - 1.0;\n+\n+                let mut scale = high - low;\n+\n+                while scale * max_rand + low >= high {\n+                    scale = <$ty>::from_bits(scale.to_bits() - 1);\n                 }\n+\n+                debug_assert!(scale >= 0.0);\n+\n+                UniformFloat { low, scale }\n             }\n \n             fn new_inclusive(low: Self::X, high: Self::X) -> Self {\n-                assert!(low <= high,\n-                        \"Uniform::new_inclusive called with `low > high`\");\n-                let scale = high - low;\n-                let offset = low - scale;\n-                UniformFloat {\n-                    scale: scale,\n-                    offset: offset,\n+                assert!(low <= high, \"Uniform::new_inclusive called with `low > high`\");\n+                assert!(low.is_finite() && high.is_finite(), \"Uniform::new_inclusive called with non-finite boundaries\");","path":"src/distributions/uniform.rs","position":85,"original_position":85,"commit_id":"8828cad87ef6ce8fae87ca158c192f27f43ac071","original_commit_id":"8828cad87ef6ce8fae87ca158c192f27f43ac071","user":{"login":"sicking","id":1831914,"node_id":"MDQ6VXNlcjE4MzE5MTQ=","avatar_url":"https://avatars2.githubusercontent.com/u/1831914?v=4","gravatar_id":"","url":"https://api.github.com/users/sicking","html_url":"https://github.com/sicking","followers_url":"https://api.github.com/users/sicking/followers","following_url":"https://api.github.com/users/sicking/following{/other_user}","gists_url":"https://api.github.com/users/sicking/gists{/gist_id}","starred_url":"https://api.github.com/users/sicking/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sicking/subscriptions","organizations_url":"https://api.github.com/users/sicking/orgs","repos_url":"https://api.github.com/users/sicking/repos","events_url":"https://api.github.com/users/sicking/events{/privacy}","received_events_url":"https://api.github.com/users/sicking/received_events","type":"User","site_admin":false},"body":"So it turns out that the old approach is actually slightly faster:\r\n\r\n```\r\n&\r\ntest gen_range_f32               ... bench:       4,459 ns/iter (+/- 169) = 897 MB/s\r\ntest gen_range_f64               ... bench:       6,027 ns/iter (+/- 334) = 1327 MB/s\r\n\r\n&&\r\ntest gen_range_f32               ... bench:       4,464 ns/iter (+/- 163) = 896 MB/s\r\ntest gen_range_f64               ... bench:       6,038 ns/iter (+/- 592) = 1324 MB/s\r\n\r\nold\r\ntest gen_range_f32               ... bench:       4,382 ns/iter (+/- 135) = 912 MB/s\r\ntest gen_range_f64               ... bench:       5,665 ns/iter (+/- 682) = 1412 MB/s\r\n```\r\n\r\nTechnically none of these differences are stasticially significant, however it definitely looks like the last is faster for f64. And it does provide better error messages.\r\n\r\nMy best guess is that the old approach ends up being implementable by using bit logic, whereas the `NEG_INF < low` checks has to be done using slower floating point instructions.","created_at":"2018-06-09T19:17:38Z","updated_at":"2018-06-09T19:17:38Z","html_url":"https://github.com/rust-lang-nursery/rand/pull/477#discussion_r194236712","pull_request_url":"https://api.github.com/repos/rust-lang-nursery/rand/pulls/477","author_association":"CONTRIBUTOR","_links":{"self":{"href":"https://api.github.com/repos/rust-lang-nursery/rand/pulls/comments/194236712"},"html":{"href":"https://github.com/rust-lang-nursery/rand/pull/477#discussion_r194236712"},"pull_request":{"href":"https://api.github.com/repos/rust-lang-nursery/rand/pulls/477"}},"in_reply_to_id":194226781},"pull_request":{"url":"https://api.github.com/repos/rust-lang-nursery/rand/pulls/477","id":190454981,"node_id":"MDExOlB1bGxSZXF1ZXN0MTkwNDU0OTgx","html_url":"https://github.com/rust-lang-nursery/rand/pull/477","diff_url":"https://github.com/rust-lang-nursery/rand/pull/477.diff","patch_url":"https://github.com/rust-lang-nursery/rand/pull/477.patch","issue_url":"https://api.github.com/repos/rust-lang-nursery/rand/issues/477","number":477,"state":"open","locked":false,"title":"Make Uniform<f32/f64> and gen_range<f32/f64> honor limits strictly. Resolves #476","user":{"login":"sicking","id":1831914,"node_id":"MDQ6VXNlcjE4MzE5MTQ=","avatar_url":"https://avatars2.githubusercontent.com/u/1831914?v=4","gravatar_id":"","url":"https://api.github.com/users/sicking","html_url":"https://github.com/sicking","followers_url":"https://api.github.com/users/sicking/followers","following_url":"https://api.github.com/users/sicking/following{/other_user}","gists_url":"https://api.github.com/users/sicking/gists{/gist_id}","starred_url":"https://api.github.com/users/sicking/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sicking/subscriptions","organizations_url":"https://api.github.com/users/sicking/orgs","repos_url":"https://api.github.com/users/sicking/repos","events_url":"https://api.github.com/users/sicking/events{/privacy}","received_events_url":"https://api.github.com/users/sicking/received_events","type":"User","site_admin":false},"body":"This adds a small amount of overhead to Uniform<f32/f64> and gen_range<f32/f64>. For Uniform<f32/f64> it adds one subtraction and one branch in the common case. For gen_range<f32/f64> it adds just one branch in the common case. I see no way around this while also being sure to stay in the requested range.\r\n\r\nThis does use f64::from_bits/to_bits/is_infinite/is_finite which might only be available through std? Though it looks like core::num also make them available [here](https://doc.rust-lang.org/stable/src/core/num/mod.rs.html#3591)? I can't tell given that the functions are marked as stable, but the trait marked unstable.\r\n\r\nI'm not sure what are ok build requirements for no_std builds? And is there any way to run `cargo test`, or even just `cargo build` in no_std mode?","created_at":"2018-05-25T00:36:01Z","updated_at":"2018-06-09T19:17:38Z","closed_at":null,"merged_at":null,"merge_commit_sha":"5bf21547f003fbeb22b5c46070245baead9ffdd1","assignee":null,"assignees":[],"requested_reviewers":[],"requested_teams":[],"labels":[{"id":958651938,"node_id":"MDU6TGFiZWw5NTg2NTE5Mzg=","url":"https://api.github.com/repos/rust-lang-nursery/rand/labels/D-changes","name":"D-changes","color":"fbca04","default":false},{"id":866995411,"node_id":"MDU6TGFiZWw4NjY5OTU0MTE=","url":"https://api.github.com/repos/rust-lang-nursery/rand/labels/D-review","name":"D-review","color":"fbca04","default":false}],"milestone":null,"commits_url":"https://api.github.com/repos/rust-lang-nursery/rand/pulls/477/commits","review_comments_url":"https://api.github.com/repos/rust-lang-nursery/rand/pulls/477/comments","review_comment_url":"https://api.github.com/repos/rust-lang-nursery/rand/pulls/comments{/number}","comments_url":"https://api.github.com/repos/rust-lang-nursery/rand/issues/477/comments","statuses_url":"https://api.github.com/repos/rust-lang-nursery/rand/statuses/8828cad87ef6ce8fae87ca158c192f27f43ac071","head":{"label":"sicking:limit_inexact_float","ref":"limit_inexact_float","sha":"8828cad87ef6ce8fae87ca158c192f27f43ac071","user":{"login":"sicking","id":1831914,"node_id":"MDQ6VXNlcjE4MzE5MTQ=","avatar_url":"https://avatars2.githubusercontent.com/u/1831914?v=4","gravatar_id":"","url":"https://api.github.com/users/sicking","html_url":"https://github.com/sicking","followers_url":"https://api.github.com/users/sicking/followers","following_url":"https://api.github.com/users/sicking/following{/other_user}","gists_url":"https://api.github.com/users/sicking/gists{/gist_id}","starred_url":"https://api.github.com/users/sicking/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sicking/subscriptions","organizations_url":"https://api.github.com/users/sicking/orgs","repos_url":"https://api.github.com/users/sicking/repos","events_url":"https://api.github.com/users/sicking/events{/privacy}","received_events_url":"https://api.github.com/users/sicking/received_events","type":"User","site_admin":false},"repo":{"id":133526105,"node_id":"MDEwOlJlcG9zaXRvcnkxMzM1MjYxMDU=","name":"rand","full_name":"sicking/rand","owner":{"login":"sicking","id":1831914,"node_id":"MDQ6VXNlcjE4MzE5MTQ=","avatar_url":"https://avatars2.githubusercontent.com/u/1831914?v=4","gravatar_id":"","url":"https://api.github.com/users/sicking","html_url":"https://github.com/sicking","followers_url":"https://api.github.com/users/sicking/followers","following_url":"https://api.github.com/users/sicking/following{/other_user}","gists_url":"https://api.github.com/users/sicking/gists{/gist_id}","starred_url":"https://api.github.com/users/sicking/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sicking/subscriptions","organizations_url":"https://api.github.com/users/sicking/orgs","repos_url":"https://api.github.com/users/sicking/repos","events_url":"https://api.github.com/users/sicking/events{/privacy}","received_events_url":"https://api.github.com/users/sicking/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/sicking/rand","description":null,"fork":true,"url":"https://api.github.com/repos/sicking/rand","forks_url":"https://api.github.com/repos/sicking/rand/forks","keys_url":"https://api.github.com/repos/sicking/rand/keys{/key_id}","collaborators_url":"https://api.github.com/repos/sicking/rand/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/sicking/rand/teams","hooks_url":"https://api.github.com/repos/sicking/rand/hooks","issue_events_url":"https://api.github.com/repos/sicking/rand/issues/events{/number}","events_url":"https://api.github.com/repos/sicking/rand/events","assignees_url":"https://api.github.com/repos/sicking/rand/assignees{/user}","branches_url":"https://api.github.com/repos/sicking/rand/branches{/branch}","tags_url":"https://api.github.com/repos/sicking/rand/tags","blobs_url":"https://api.github.com/repos/sicking/rand/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/sicking/rand/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/sicking/rand/git/refs{/sha}","trees_url":"https://api.github.com/repos/sicking/rand/git/trees{/sha}","statuses_url":"https://api.github.com/repos/sicking/rand/statuses/{sha}","languages_url":"https://api.github.com/repos/sicking/rand/languages","stargazers_url":"https://api.github.com/repos/sicking/rand/stargazers","contributors_url":"https://api.github.com/repos/sicking/rand/contributors","subscribers_url":"https://api.github.com/repos/sicking/rand/subscribers","subscription_url":"https://api.github.com/repos/sicking/rand/subscription","commits_url":"https://api.github.com/repos/sicking/rand/commits{/sha}","git_commits_url":"https://api.github.com/repos/sicking/rand/git/commits{/sha}","comments_url":"https://api.github.com/repos/sicking/rand/comments{/number}","issue_comment_url":"https://api.github.com/repos/sicking/rand/issues/comments{/number}","contents_url":"https://api.github.com/repos/sicking/rand/contents/{+path}","compare_url":"https://api.github.com/repos/sicking/rand/compare/{base}...{head}","merges_url":"https://api.github.com/repos/sicking/rand/merges","archive_url":"https://api.github.com/repos/sicking/rand/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/sicking/rand/downloads","issues_url":"https://api.github.com/repos/sicking/rand/issues{/number}","pulls_url":"https://api.github.com/repos/sicking/rand/pulls{/number}","milestones_url":"https://api.github.com/repos/sicking/rand/milestones{/number}","notifications_url":"https://api.github.com/repos/sicking/rand/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/sicking/rand/labels{/name}","releases_url":"https://api.github.com/repos/sicking/rand/releases{/id}","deployments_url":"https://api.github.com/repos/sicking/rand/deployments","created_at":"2018-05-15T14:11:50Z","updated_at":"2018-06-08T19:58:38Z","pushed_at":"2018-06-09T18:39:16Z","git_url":"git://github.com/sicking/rand.git","ssh_url":"git@github.com:sicking/rand.git","clone_url":"https://github.com/sicking/rand.git","svn_url":"https://github.com/sicking/rand","homepage":"https://docs.rs/rand","size":85974,"stargazers_count":0,"watchers_count":0,"language":"Rust","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"archived":false,"open_issues_count":0,"license":{"key":"other","name":"Other","spdx_id":null,"url":null,"node_id":"MDc6TGljZW5zZTA="},"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"rust-lang-nursery:master","ref":"master","sha":"276c8be320710330ad51a72f304f16475c8ae8cc","user":{"login":"rust-lang-nursery","id":14631425,"node_id":"MDEyOk9yZ2FuaXphdGlvbjE0NjMxNDI1","avatar_url":"https://avatars1.githubusercontent.com/u/14631425?v=4","gravatar_id":"","url":"https://api.github.com/users/rust-lang-nursery","html_url":"https://github.com/rust-lang-nursery","followers_url":"https://api.github.com/users/rust-lang-nursery/followers","following_url":"https://api.github.com/users/rust-lang-nursery/following{/other_user}","gists_url":"https://api.github.com/users/rust-lang-nursery/gists{/gist_id}","starred_url":"https://api.github.com/users/rust-lang-nursery/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rust-lang-nursery/subscriptions","organizations_url":"https://api.github.com/users/rust-lang-nursery/orgs","repos_url":"https://api.github.com/users/rust-lang-nursery/repos","events_url":"https://api.github.com/users/rust-lang-nursery/events{/privacy}","received_events_url":"https://api.github.com/users/rust-lang-nursery/received_events","type":"Organization","site_admin":false},"repo":{"id":30227790,"node_id":"MDEwOlJlcG9zaXRvcnkzMDIyNzc5MA==","name":"rand","full_name":"rust-lang-nursery/rand","owner":{"login":"rust-lang-nursery","id":14631425,"node_id":"MDEyOk9yZ2FuaXphdGlvbjE0NjMxNDI1","avatar_url":"https://avatars1.githubusercontent.com/u/14631425?v=4","gravatar_id":"","url":"https://api.github.com/users/rust-lang-nursery","html_url":"https://github.com/rust-lang-nursery","followers_url":"https://api.github.com/users/rust-lang-nursery/followers","following_url":"https://api.github.com/users/rust-lang-nursery/following{/other_user}","gists_url":"https://api.github.com/users/rust-lang-nursery/gists{/gist_id}","starred_url":"https://api.github.com/users/rust-lang-nursery/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rust-lang-nursery/subscriptions","organizations_url":"https://api.github.com/users/rust-lang-nursery/orgs","repos_url":"https://api.github.com/users/rust-lang-nursery/repos","events_url":"https://api.github.com/users/rust-lang-nursery/events{/privacy}","received_events_url":"https://api.github.com/users/rust-lang-nursery/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/rust-lang-nursery/rand","description":"A Rust library for random number generation.","fork":false,"url":"https://api.github.com/repos/rust-lang-nursery/rand","forks_url":"https://api.github.com/repos/rust-lang-nursery/rand/forks","keys_url":"https://api.github.com/repos/rust-lang-nursery/rand/keys{/key_id}","collaborators_url":"https://api.github.com/repos/rust-lang-nursery/rand/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/rust-lang-nursery/rand/teams","hooks_url":"https://api.github.com/repos/rust-lang-nursery/rand/hooks","issue_events_url":"https://api.github.com/repos/rust-lang-nursery/rand/issues/events{/number}","events_url":"https://api.github.com/repos/rust-lang-nursery/rand/events","assignees_url":"https://api.github.com/repos/rust-lang-nursery/rand/assignees{/user}","branches_url":"https://api.github.com/repos/rust-lang-nursery/rand/branches{/branch}","tags_url":"https://api.github.com/repos/rust-lang-nursery/rand/tags","blobs_url":"https://api.github.com/repos/rust-lang-nursery/rand/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/rust-lang-nursery/rand/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/rust-lang-nursery/rand/git/refs{/sha}","trees_url":"https://api.github.com/repos/rust-lang-nursery/rand/git/trees{/sha}","statuses_url":"https://api.github.com/repos/rust-lang-nursery/rand/statuses/{sha}","languages_url":"https://api.github.com/repos/rust-lang-nursery/rand/languages","stargazers_url":"https://api.github.com/repos/rust-lang-nursery/rand/stargazers","contributors_url":"https://api.github.com/repos/rust-lang-nursery/rand/contributors","subscribers_url":"https://api.github.com/repos/rust-lang-nursery/rand/subscribers","subscription_url":"https://api.github.com/repos/rust-lang-nursery/rand/subscription","commits_url":"https://api.github.com/repos/rust-lang-nursery/rand/commits{/sha}","git_commits_url":"https://api.github.com/repos/rust-lang-nursery/rand/git/commits{/sha}","comments_url":"https://api.github.com/repos/rust-lang-nursery/rand/comments{/number}","issue_comment_url":"https://api.github.com/repos/rust-lang-nursery/rand/issues/comments{/number}","contents_url":"https://api.github.com/repos/rust-lang-nursery/rand/contents/{+path}","compare_url":"https://api.github.com/repos/rust-lang-nursery/rand/compare/{base}...{head}","merges_url":"https://api.github.com/repos/rust-lang-nursery/rand/merges","archive_url":"https://api.github.com/repos/rust-lang-nursery/rand/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/rust-lang-nursery/rand/downloads","issues_url":"https://api.github.com/repos/rust-lang-nursery/rand/issues{/number}","pulls_url":"https://api.github.com/repos/rust-lang-nursery/rand/pulls{/number}","milestones_url":"https://api.github.com/repos/rust-lang-nursery/rand/milestones{/number}","notifications_url":"https://api.github.com/repos/rust-lang-nursery/rand/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/rust-lang-nursery/rand/labels{/name}","releases_url":"https://api.github.com/repos/rust-lang-nursery/rand/releases{/id}","deployments_url":"https://api.github.com/repos/rust-lang-nursery/rand/deployments","created_at":"2015-02-03T05:59:32Z","updated_at":"2018-06-09T15:33:19Z","pushed_at":"2018-06-09T18:49:08Z","git_url":"git://github.com/rust-lang-nursery/rand.git","ssh_url":"git@github.com:rust-lang-nursery/rand.git","clone_url":"https://github.com/rust-lang-nursery/rand.git","svn_url":"https://github.com/rust-lang-nursery/rand","homepage":"https://crates.io/crates/rand","size":85985,"stargazers_count":213,"watchers_count":213,"language":"Rust","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":true,"forks_count":141,"mirror_url":null,"archived":false,"open_issues_count":50,"license":{"key":"other","name":"Other","spdx_id":null,"url":null,"node_id":"MDc6TGljZW5zZTA="},"forks":141,"open_issues":50,"watchers":213,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/rust-lang-nursery/rand/pulls/477"},"html":{"href":"https://github.com/rust-lang-nursery/rand/pull/477"},"issue":{"href":"https://api.github.com/repos/rust-lang-nursery/rand/issues/477"},"comments":{"href":"https://api.github.com/repos/rust-lang-nursery/rand/issues/477/comments"},"review_comments":{"href":"https://api.github.com/repos/rust-lang-nursery/rand/pulls/477/comments"},"review_comment":{"href":"https://api.github.com/repos/rust-lang-nursery/rand/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/rust-lang-nursery/rand/pulls/477/commits"},"statuses":{"href":"https://api.github.com/repos/rust-lang-nursery/rand/statuses/8828cad87ef6ce8fae87ca158c192f27f43ac071"}},"author_association":"CONTRIBUTOR"}}
{"actor":{"display_login":"sicking"}}