Opened 8 weeks ago

Last modified 3 weeks ago

#30615 needs_revision enhancement

Factor random_uniform_01 into nondeterministic and deterministic parts, and automatically test the deterministic part

Reported by: riastradh Owned by:
Priority: Low Milestone: Tor: 0.4.2.x-final
Component: Core Tor/Tor Version: Tor: 0.4.1.1-alpha
Severity: Minor Keywords: tor-test, 041-deferred-20190530
Cc: Actual Points:
Parent ID: Points:
Reviewer: asn Sponsor:

Description

The random_uniform_01 code is not automatically tested by any deterministic tests -- it is tested only by the stochastic tests in the slow test suite. However, there is a not-entirely-trivial deterministic computation inside it, wired up to a (truncated) geometric(1/2) sampler and a uniform bit sampler. And it turns out that this deterministic computation technically has a bug, although the bug triggers with probability <2-900 so it doesn't really matter. But it is nice to have deterministic tests for correct behaviour of the deterministic part of a random sampler, and nice for that deterministic part to not have bugs.

Child Tickets

Attachments (3)

test-uniform01-ldexp.patch (6.7 KB) - added by riastradh 8 weeks ago.
test-uniform01-ldexp-v2.patch (7.0 KB) - added by riastradh 6 weeks ago.
fix tabs, add comment
test-uniform01-ldexp-v3.patch (7.0 KB) - added by riastradh 6 weeks ago.
apparently Windows doesn't understand %zu for size_t

Download all attachments as: .zip

Change History (12)

Changed 8 weeks ago by riastradh

Attachment: test-uniform01-ldexp.patch added

comment:1 Changed 8 weeks ago by nickm

Milestone: Tor: 0.4.1.x-final
Status: newneeds_review

comment:2 Changed 8 weeks ago by dgoulet

Keywords: tor-test added
Reviewer: asn

comment:3 Changed 7 weeks ago by nickm

Keywords: 041-deferred-20190530 added

Marking these tickets as deferred from 041.

comment:4 Changed 7 weeks ago by nickm

Milestone: Tor: 0.4.1.x-finalTor: unspecified

comment:5 Changed 7 weeks ago by nickm

Milestone: Tor: unspecifiedTor: 0.4.2.x-final

comment:6 Changed 7 weeks ago by asn

Status: needs_reviewneeds_revision

Can we have some more comments on what the test actually does, in particular what is this supposed to do in the loop:
double y = sample_uniform_01(e, hi ^ (hihi << 31), lo ^ lolo);

Thanks!

I also opened a PR so that the CI runs: https://github.com/torproject/tor/pull/1069

Changed 6 weeks ago by riastradh

fix tabs, add comment

comment:7 Changed 6 weeks ago by asn

Hey Riastradh,

thanks for the comments

there also seems to be a broken appveyor build due to some compilation errors: https://ci.appveyor.com/project/torproject/tor/builds/24996783/job/2kxtj1cd4m773iyy

Changed 6 weeks ago by riastradh

apparently Windows doesn't understand %zu for size_t

comment:8 Changed 3 weeks ago by asn

Status: needs_revisionneeds_review

comment:9 Changed 3 weeks ago by asn

Status: needs_reviewneeds_revision

Thanks for the patch Riastradh. I pushed your merge with a changes file in https://github.com/torproject/tor/pull/1069.

It seems like appveyor is failing the new deterministic unit test on Windows. Haven't had time to debug this yet. https://ci.appveyor.com/project/torproject/tor/builds/25689273/job/ra125nk52t2lkj85

Note: See TracTickets for help on using tickets.