Samasaur1 / DiceKit

A Swift module for simulating dice both real and unreal
https://samasaur1.github.io/DiceKit/
Academic Free License v3.0
4 stars 4 forks source link

Version 0.17.0: Chance and Probabilities #62

Closed Samasaur1 closed 5 years ago

Samasaur1 commented 5 years ago

Here's what's new:

Here's what has to happen:

Samasaur1 commented 5 years ago

The changelog still needs the date of this upcoming release — which hasn't been determined yet!

Samasaur1 commented 5 years ago

One of the Travis tests failed:

Test Case '-[DiceKitTests.DiceKitTests testRollable]' started.

/Users/travis/build/Samasaur1/DiceKit/Tests/DiceKitTests/DiceKitTests.swift:46: error: -[DiceKitTests.DiceKitTests testRollable] : XCTAssertGreaterThan failed: ("0.0") is not greater than ("0.0") - 

Test Case '-[DiceKitTests.DiceKitTests testRollable]' failed (9.866 seconds).

Test Suite 'DiceKitTests' failed at 2019-07-03 16:05:58.532.

     Executed 7 tests, with 1 failure (0 unexpected) in 9.894 (9.902) seconds

I don't know what type of Rollable it was that failed, but the probabilities of the roll that was rolled was 0.0 — which should be impossible

Samasaur1 commented 5 years ago

Note that a much too large number of Travis build cases are stalling on testRollable(). This clearly needs to be reworked — it can't be removed, because it has also failed, and I need to know how.

Samasaur1 commented 5 years ago

I'm having to restart basically every 1 out of 3 builds because of Travis thinking it stalled or it failing with Signal code 4

Samasaur1 commented 5 years ago

As of right now (I believe), any Travis build that says it has failed has actually failed because of testRollable(), saying 0.0 is not greater than 0.0, on the same line. It's happened across different versions of Swift and Xcode, on both macOS and Linux.

The next step obviously needs to be to find the problem causing this.

Samasaur1 commented 5 years ago

The PR and branch builds (a total of 12) errored 5 times. Five times — and they were all on testDiceRollable. However, that's because of stalling, not the same failing error.

Samasaur1 commented 5 years ago

The (read: a) problem is in the getPartitions(ofSize:forTarget:) function, which isn't returning (in this example) [25, 25, 4] for getPartitions(ofSize: 3, forTarget: 54). There are other ones that should be returned and are not. This is just one example.

Samasaur1 commented 5 years ago

The Chance.+(_:_:) function, here, can overflow on either the lnum or rnum multiplication operations. This occurs when the n and/or d properties are very large because of the double approximation initializer. Dividing before multiplying by lcm does not work, as n/d then becomes 0.

static func + (lhs: Chance, rhs: Chance) -> Chance {
    let lcm = Chance.lcm(lhs.d, rhs.d)
    let lnum = lhs.n * lcm / lhs.d
    let rnum = rhs.n * lcm / rhs.d
    return (try? .init(lnum + rnum, outOf: lcm)) ?? .one
}

I'm not sure how to move forward on this.

Samasaur1 commented 5 years ago

This errored when calculating probabilities by running out of time, so that problem wasn't fixed...

Samasaur1 commented 5 years ago

I just added these memoize functions, but we're basically doing that already

Samasaur1 commented 5 years ago

I think that last commit (7d4a172259167286603bbb7da9a00862549c5807) fixes all the Dice timing out errors

Samasaur1 commented 5 years ago

Apparently 7fb5a024686c7b0fcac6ec496c13b0d6ec6ef2a6 broke Linux

Samasaur1 commented 5 years ago

Also, we're still getting the Chance.+(_:_:) overflow problem, which may be causing the Signal code 4 error

Samasaur1 commented 5 years ago

That Double.random call was something that no sane person (looks around guiltily) would blame me for — and it was in a test, so I feel fine in taking it out

Samasaur1 commented 5 years ago

I had stopped using the getRollable(die:) method, but updated that instead of the actual way that the test generated a WeightedDie

Samasaur1 commented 5 years ago

There are 6 more tests that need to be implemented:

Notice that 5/6 of them should have been written before this version, because they don't test new features

Samasaur1 commented 5 years ago

The sixth one has been implemented, which leaves the 5 that already should have been. I'm going to leave those as stubs because they weren't introduced in this version