Quantum and atmospheric randomness should be 1-based

larkery commented

2023-05-23 19:44:18 +00:00

(Migrated from github.com)

Add 1, as with calls to (random)

zzkt commented

2023-05-24 07:42:19 +00:00

(Migrated from github.com)

Thanks for the patch. I assume you got an out of range error somewhere? The current ert tests probably won't catch it, so can you let me know how it surfaced?

The i-ching-r64 function should return a number from 1 to 64. The request params are ?num=1&min=1&max=64&base=10 if you are seeing an off-by-1 there, it could be the API call and would be useful to see a test case.

The i-ching-q64 looks like it could be returning 0-63 but I'll have to take a closer look (requests are rate limited)

Thanks for the patch. I assume you got an out of range error somewhere? The current `ert` tests probably won't catch it, so can you let me know how it surfaced? The `i-ching-r64` function should return a number from 1 to 64. The request params are `?num=1&min=1&max=64&base=10` if you are seeing an off-by-1 there, it could be the API call and would be useful to see a test case. The `i-ching-q64` looks like it could be returning 0-63 but I'll have to take a closer look (requests are rate limited)

zzkt commented

2023-05-24 16:43:00 +00:00

(Migrated from github.com)

looks like i-ching-r64 is behaving as expected (after a few thousand API calls..)

	rate (n=8475 p=0.0155)
0	0
1	0.0159
2	0.0168
63	0.0151
64	0.0151
65	0

looks like `i-ching-r64` is behaving as expected (after a few thousand API calls..) | | rate (n=8475 p=0.0155) | |----|----| | 0 | 0 | | 1 | 0.0159 | | 2 | 0.0168 | | 63 | 0.0151 | | 64 | 0.0151 | | 65 | 0 |

zzkt commented

2023-05-25 08:08:39 +00:00

(Migrated from github.com)

on the other had i-ching-q64...

{"type":"string","length":1,"size":1,"data":["00"],"success":true}

on the other had `i-ching-q64`... `{"type":"string","length":1,"size":1,"data":["00"],"success":true}`

larkery commented

2023-05-25 09:30:47 +00:00

(Migrated from github.com)

Interesting; on my machine I observe i-ching--three-coins sometimes returning nil with either atmospheric or quantum randomness. The path is something like this:

i-ching--coin-toss
- (+ 1 (i-ching-random 2))
  - (/ (i-ching-r64) (/ 64 2))
    - (/ (i-ching-r64) 32)
      - (i-ching-r64) => 20
    - (/ 20 32) => 0 (this is where the fault is)
  - (+ 1 0) => 1

do this 3 times and 'result' in i-ching--three-coins is 3, which makes the pcase in there return nil.

I think the behaviour is different if you call (i-ching-random 64) (if the contract on q-64 or r-64 is 1-based) than if you call with n /= 64. The integer division in the other branches (where n isn't 64) will always return zero - n-1 rather than 1-n, because f.e. (/ 1 4) => 0. I might be missing something though?

Another thing that could go in i-ching-random would be

(defun i-ching-random (n &optional source)
  (+ 1
     (mod
      (pcase (or source i-ching-randomness-source)
        ('quantum (- (i-ching-q64) 1))
        ('atmospheric (- (i-ching-r64) 1))
        (_ (random n)))
      n)))

Which would guarantee an outcome that is in range so long as the randomness source does return an integer.

Interesting; on my machine I observe i-ching--three-coins sometimes returning nil with either atmospheric or quantum randomness. The path is something like this: - i-ching--coin-toss - (+ 1 (i-ching-random 2)) - (/ (i-ching-r64) (/ 64 2)) - (/ (i-ching-r64) 32) - (i-ching-r64) => 20 - (/ 20 32) => 0 (this is where the fault is) - (+ 1 0) => 1 do this 3 times and 'result' in i-ching--three-coins is 3, which makes the pcase in there return nil. I think the behaviour is different if you call (i-ching-random 64) (if the contract on q-64 or r-64 is 1-based) than if you call with n /= 64. The integer division in the other branches (where n isn't 64) will always return zero - n-1 rather than 1-n, because f.e. (/ 1 4) => 0. I might be missing something though? Another thing that could go in i-ching-random would be ``` (defun i-ching-random (n &optional source) (+ 1 (mod (pcase (or source i-ching-randomness-source) ('quantum (- (i-ching-q64) 1)) ('atmospheric (- (i-ching-r64) 1)) (_ (random n))) n))) ``` Which would guarantee an outcome that is in range so long as the randomness source does return an integer.

larkery commented

2023-05-25 09:33:35 +00:00

(Migrated from github.com)

Ah so thinking about this more my diagnosis was wrong and the code I suggest above is a proper fix; it's the division not the 1- or 0- basis of the methods.

zzkt commented

2023-05-26 11:16:56 +00:00

(Migrated from github.com)

Thanks again @larkery it should be fixed with ab9eaa4

Thanks again @larkery it should be fixed with [ab9eaa4](https://github.com/zzkt/i-ching/commit/ab9eaa445d21349213043298bbfe07e81c20a2e3)

Pull request closed

This pull request cannot be reopened because the branch was deleted.

Quantum and atmospheric randomness should be 1-based #9

Pull request closed