Opened 6 months ago

Closed 4 weeks ago

#28878 closed defect (fixed)

WTF-PAD: Improve deterministic randomness in tests

Reported by: asn Owned by: nickm
Priority: Low Milestone: Tor: 0.4.1.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: wtf-pad, tor-relay, tor-cell, padding, tor-tests, dgoulet-merge, 041-should
Cc: Actual Points: .2
Parent ID: Points: 2
Reviewer: Sponsor: Sponsor2

Description (last modified by teor)

As part of test_prob_distr.c in #28142, we implemented a deterministic randomness system for the stochastic tests. There are various things we can improve there:

a) Make it into its own subsystem so that other tests can also use it.
b) Don't use a uint32_t counter as the seed, because the byte order is machine dependent. Instead make it a uint8_t[4] or something.
c) Allow users to overwrite the seed using environment variables or the CLI. For now users have to tweak the init_deterministic_rand() func.

Child Tickets

Change History (18)

comment:1 Changed 6 months ago by asn

Summary: Improve deterministic randomness in testsWTF-PAD: Improve deterministic randomness in tests

comment:2 Changed 6 months ago by asn

< Riastradh> asn: How to increment a counter in a machine-independent way:  unsigned i, t = 1; for (i = 0; i < arraycount(ctr); i++) { t = ctr[i] + 1; ctr[i] = t 
             & 0xff; t >>= 8; }
< Riastradh> (You can assert(t == 0) at the end, but there's really no need if arraycount(ctr) >= 8 and ctr starts at zero.)
< Riastradh> asn: Tiny nit in the comments: change `set the expected log-probability' to `set the log-probability of the null hypothesis' or `set logP to the 
             logarithm of the expected frequency under the null hypothesis'.
< Riastradh> asn: In particular, log E[X] generally not E[log X].  E.g., if P(0) = 1/2, P(1) = 1/4, P(2) = P(3) = 1/8, then log E[P(x)] = log 11/32 ~= -1.07, but 
             E[log P(x)] ~= -1.21.
< Riastradh> asn: How to paint a 32-bike shed:  for (i = 0; i < 32; i++) { paintf("%02x", shed[i]); }
< Riastradh> asn: Correction to my off-the-cuff IRC code: not { t = ctr[i] + 1; ...} but { t = ctr[i] + t; ... }, or just { t += ctr[i]; ... }.

comment:3 Changed 5 months ago by mikeperry

Keywords: 041-proposed added

comment:4 Changed 5 months ago by mikeperry

Points: 2

comment:5 Changed 5 months ago by mikeperry

Priority: MediumLow

comment:6 Changed 5 months ago by nickm

Sponsor: Sponsor2

comment:7 Changed 5 months ago by nickm

Parent ID: #28637#28631

Reparent children of #28637 into #28631

comment:8 Changed 4 months ago by mikeperry

Keywords: 041-proposed removed

Take some wtf-pad stuff out of 041-proposed

comment:9 Changed 3 months ago by teor

Description: modified (diff)

Fix calculation

comment:10 Changed 3 months ago by nickm

See #29732 for a generic backend here.

comment:11 Changed 5 weeks ago by mikeperry

Parent ID: #28631

comment:12 Changed 5 weeks ago by nickm

Actual Points: .2
Milestone: Tor: unspecifiedTor: 0.4.1.x-final
Owner: set to nickm
Status: newaccepted

See branch rng_coverage. (It is based on a merge of bug30475_035, since I need that for my GCC to work.)

comment:13 Changed 5 weeks ago by nickm

Status: acceptedneeds_review

comment:14 Changed 5 weeks ago by nickm

(I've rebased this branch on master, now that #30475 is merged)

comment:15 Changed 5 weeks ago by asn

Status: needs_reviewmerge_ready

LGTM! Also tested it works on failed tests.

comment:16 Changed 5 weeks ago by nickm

Keywords: dgoulet-merge added

comment:17 Changed 5 weeks ago by nickm

Keywords: 041-should added

comment:18 Changed 4 weeks ago by dgoulet

Resolution: fixed
Status: merge_readyclosed

Merged!

Note: See TracTickets for help on using tickets.