Skip to content

Commit 2181913

Browse files
pupnewfsterpupnewfster
committed
Replace QuickTheories with jqwik as it is a property based testing lib that is still receiving updates, and fix some edge cases where FloatingLong's did not perform certain calculations properly
1 parent 1d19819 commit 2181913

File tree

5 files changed

+243
-181
lines changed

5 files changed

+243
-181
lines changed

‎build.gradle

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -378,8 +378,8 @@ dependencies {
378378
testImplementation(testFrameWork)
379379
testImplementation("org.junit.jupiter:junit-jupiter-api:${junit_version}")
380380
testRuntimeOnly("org.junit.jupiter:junit-jupiter-engine:${junit_version}")
381-
//We use https://github.com/quicktheories/QuickTheories to allow for implementing property based testing
382-
testImplementation("org.quicktheories:quicktheories:${quicktheories_version}")
381+
//We use https://github.com/jqwik-team/jqwik to allow for implementing property based testing
382+
testImplementation("net.jqwik:jqwik:${jqwik_version}")
383383

384384
compileOnly("mezz.jei:jei-${previous_minor_minecraft_version}-common-api:${jei_version}")
385385
compileOnly("mezz.jei:jei-${previous_minor_minecraft_version}-neoforge-api:${jei_version}")

‎gradle.properties

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ release_type=beta
2121

2222
#JUnit/Testing dependencies
2323
junit_version=5.10.2
24-
quicktheories_version=0.26
24+
jqwik_version=1.8.5
2525

2626
#NeoGradle Settings
2727
neogradle.subsystems.parchment.minecraftVersion=1.20.6

‎src/api/java/mekanism/api/math/FloatingLong.java

Lines changed: 98 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import mekanism.api.annotations.NothingNullByDefault;
1616
import net.minecraft.network.codec.ByteBufCodecs;
1717
import net.minecraft.network.codec.StreamCodec;
18+
import org.jetbrains.annotations.VisibleForTesting;
1819

1920
/**
2021
* A class representing a positive number with an internal value defined by an unsigned long, and a floating point number stored in a short
@@ -53,7 +54,8 @@ public <T> T write(DynamicOps<T> ops, FloatingLong value) {
5354
/**
5455
* The maximum number of decimal digits we can represent
5556
*/
56-
private static final int DECIMAL_DIGITS = 4;
57+
@VisibleForTesting
58+
static final int DECIMAL_DIGITS = 4;
5759
/**
5860
* The maximum value we can represent as a decimal
5961
*/
@@ -62,10 +64,14 @@ public <T> T write(DynamicOps<T> ops, FloatingLong value) {
6264
* The value which represents 1.0, this is one more than the value of {@link #MAX_DECIMAL}
6365
*/
6466
private static final short SINGLE_UNIT = MAX_DECIMAL + 1;
67+
/**
68+
* Used for calculating whether a multiplication will overflow
69+
*/
70+
private static final long LONG_SHIFT = Long.divideUnsigned(-1, SINGLE_UNIT);
6571
/**
6672
* The maximum value where the decimal can be eliminated without {@link #value} overflowing, want to be able to shift twice
6773
*/
68-
private static final long MAX_LONG_SHIFT = Long.divideUnsigned(Long.divideUnsigned(-1L, SINGLE_UNIT), SINGLE_UNIT);
74+
private static final long MAX_LONG_SHIFT = Long.divideUnsigned(LONG_SHIFT, SINGLE_UNIT);
6975
/**
7076
* A constant holding the value {@code 0}
7177
*/
@@ -82,6 +88,8 @@ public <T> T write(DynamicOps<T> ops, FloatingLong value) {
8288
* A constant holding the maximum value for a {@link FloatingLong} represented as a double
8389
*/
8490
private static final double MAX_AS_DOUBLE = Double.parseDouble(MAX_VALUE.toString());
91+
private static final int MAX_UNSIGNED_LENGTH = Long.toUnsignedString(-1).length();
92+
private static final BigInteger MAX_UNSIGNED_LONG = new BigInteger(Long.toUnsignedString(-1));
8593

8694
/**
8795
* Creates a mutable {@link FloatingLong} from a given primitive double.
@@ -294,6 +302,8 @@ public FloatingLong plusEqual(FloatingLong toAdd) {
294302
private FloatingLong plusEqual(long toAddValue, short toAddDecimal) {
295303
if (toAddDecimal == 0) {
296304
return plusEqual(toAddValue);
305+
} else if (isZero()) {
306+
return setAndClampValues(toAddValue, toAddDecimal);
297307
} else if ((value < 0 && toAddValue < 0) || ((value < 0 || toAddValue < 0) && (value + toAddValue >= 0))) {
298308
//To save a tiny bit of memory if this is called on a constant we just return the constant max as the object can't be modified anyway
299309
return isConstant ? MAX_VALUE : setAndClampValues(-1, MAX_DECIMAL);
@@ -327,6 +337,8 @@ private FloatingLong plusEqual(long toAddValue, short toAddDecimal) {
327337
public FloatingLong plusEqual(long toAdd) {
328338
if (toAdd == 0) {
329339
return this;
340+
} else if (isZero()) {
341+
return setAndClampValues(toAdd, decimal);
330342
} else if ((value < 0 && toAdd < 0) || ((value < 0 || toAdd < 0) && (value + toAdd >= 0))) {
331343
//To save a tiny bit of memory if this is called on a constant we just return the constant max as the object can't be modified anyway
332344
return isConstant ? MAX_VALUE : setAndClampValues(-1, MAX_DECIMAL);
@@ -487,8 +499,18 @@ public FloatingLong divideEquals(FloatingLong toDivide) {
487499
return divideEquals(toDivide.value);
488500
}
489501
BigDecimal divide = new BigDecimal(toString()).divide(new BigDecimal(toDivide.toString()), DECIMAL_DIGITS, RoundingMode.HALF_UP);
490-
long value = divide.longValue();
491-
short decimal = parseDecimal(divide.toPlainString());
502+
String plainString = divide.toPlainString();
503+
int decimalIndex = plainString.indexOf('.');
504+
int length = decimalIndex == -1 ? plainString.length() : decimalIndex;
505+
long value;
506+
//If we are less than max floating long, just grab the long value from the big decimal
507+
if (length < MAX_UNSIGNED_LENGTH || length == MAX_UNSIGNED_LENGTH && divide.toBigInteger().compareTo(MAX_UNSIGNED_LONG) <= 0) {
508+
value = divide.longValue();
509+
} else {
510+
//Otherwise clamp it to the max value
511+
return isConstant ? MAX_VALUE : setAndClampValues(-1, MAX_DECIMAL);
512+
}
513+
short decimal = parseDecimal(plainString, decimalIndex);
492514
return setAndClampValues(value, decimal);
493515
}
494516

@@ -510,6 +532,8 @@ public FloatingLong divideEquals(long toDivide) {
510532
throw new ArithmeticException("Division by zero");
511533
} else if (isZero() || toDivide == 1) {
512534
return this;
535+
} else if (decimal == 0 && this.value == toDivide) {
536+
return isConstant ? ONE : setAndClampValues(1, (short) 0);
513537
}
514538
long val = Long.divideUnsigned(this.value, toDivide);
515539
long rem = Long.remainderUnsigned(this.value, toDivide);
@@ -522,19 +546,30 @@ public FloatingLong divideEquals(long toDivide) {
522546
//if that'll overflow, then toDivide also has to be big. let's just lose some denominator precision and use that
523547
dec = Long.divideUnsigned(rem, Long.divideUnsigned(toDivide, SINGLE_UNIT * 10L)); //same as multiplying numerator
524548
} else {
525-
dec = Long.divideUnsigned(rem * SINGLE_UNIT * 10L, toDivide); //trivial case
526-
dec += Long.divideUnsigned(this.decimal * 10L, toDivide); //need to account for dividing decimal too in case toDivide < 10k
549+
//need to account for dividing decimal too in case toDivide < 10k
550+
if (this.decimal == 0 || Long.compareUnsigned(rem, MAX_LONG_SHIFT / 100) >= 0) {
551+
dec = Long.divideUnsigned(rem * SINGLE_UNIT * 10L, toDivide); //trivial case
552+
long additional = Long.divideUnsigned(this.decimal * 10L * SINGLE_UNIT, toDivide);
553+
dec = dec + Long.divideUnsigned(additional + 5, 10) + 1;
554+
} else if (Long.compareUnsigned(rem, MAX_LONG_SHIFT / 1_000) >= 0) {
555+
dec = Long.divideUnsigned(rem * SINGLE_UNIT * 100, toDivide); //trivial case
556+
long additional = Long.divideUnsigned(this.decimal * 10L * SINGLE_UNIT, toDivide);
557+
dec = Long.divideUnsigned(dec + Long.divideUnsigned(additional + 5, 10) + 1, 10);
558+
} else if (Long.compareUnsigned(rem, MAX_LONG_SHIFT / SINGLE_UNIT) >= 0) {
559+
dec = Long.divideUnsigned(rem * SINGLE_UNIT * 1_000, toDivide); //trivial case
560+
long additional = Long.divideUnsigned(this.decimal * 10L * SINGLE_UNIT, toDivide);
561+
dec = Long.divideUnsigned(dec + Long.divideUnsigned(additional + 5, 10) + 1, 100);
562+
} else {
563+
dec = Long.divideUnsigned(rem * SINGLE_UNIT * SINGLE_UNIT, toDivide); //trivial case
564+
long additional = Long.divideUnsigned(this.decimal * 10L * SINGLE_UNIT, toDivide);
565+
dec = Long.divideUnsigned(dec + Long.divideUnsigned(additional + 5, 10) + 1, 1_000);
566+
}
527567
}
528568

529569
//usually will expect to round to nearest, so we have to do that here
530-
if (Long.remainderUnsigned(dec, 10) >= 5) {
531-
dec += 10;
532-
if (dec >= SINGLE_UNIT * 10) { //round up + carry over to val
533-
val++;
534-
dec -= SINGLE_UNIT * 10;
535-
}
536-
}
537-
dec /= 10;
570+
dec = Long.divideUnsigned(dec + 5, 10);
571+
val += Long.divideUnsigned(dec, SINGLE_UNIT);
572+
dec = Long.remainderUnsigned(dec, SINGLE_UNIT);
538573
return setAndClampValues(val, (short) dec);
539574
}
540575

@@ -553,40 +588,73 @@ public long divideToUnsignedLong(FloatingLong toDivide) {
553588
} else if (toDivide.equals(ONE)) {
554589
//Directly return our value if we are dividing by one
555590
return value;
556-
} else if (smallerThan(toDivide)) {
591+
}
592+
int comparison = compareTo(toDivide);
593+
if (comparison < 0) {
557594
// Return early if operation will return < 1
558595
return 0;
596+
} else if (comparison == 0) {
597+
//x / x is always equal to one
598+
return 1;
559599
}
560600
if (toDivide.greaterThan(ONE)) {
561601
//If toDivide > 1, then we don't care about this.decimal, so can optimize out accounting for that
562602
if (Long.compareUnsigned(toDivide.value, MAX_LONG_SHIFT) <= 0) { //don't case if *this* is < or > than shift
563603
long div = toDivide.value * SINGLE_UNIT + toDivide.decimal;
564-
return (Long.divideUnsigned(this.value, div) * SINGLE_UNIT) + Long.divideUnsigned(Long.remainderUnsigned(this.value, div) * SINGLE_UNIT, div);
604+
return (Long.divideUnsigned(this.value, div) * SINGLE_UNIT) + Long.divideUnsigned(this.decimal + Long.remainderUnsigned(this.value, div) * SINGLE_UNIT, div);
565605
}
566606
// we already know toDivide is > max_long_shift, and other case is impossible
567607
if (Long.compareUnsigned(toDivide.value, Long.divideUnsigned(-1L, 2) + 1L) >= 0) {
568608
//need to check anyway to avoid overflow on toDivide.value +1, so might as well return early
569609
return 1;
570610
}
571611
long q = Long.divideUnsigned(this.value, toDivide.value);
572-
if (q != Long.divideUnsigned(this.value, toDivide.value + 1)) {
612+
if (toDivide.decimal > 0 && q != Long.divideUnsigned(this.value, toDivide.value + 1)) {
573613
// check if we need to account for toDivide.decimal in this case
574-
if (toDivide.value * q + Long.divideUnsigned(toDivide.decimal * q, MAX_DECIMAL) > this.value) {
614+
long decimalAdjustment = Long.divideUnsigned(toDivide.decimal * q, SINGLE_UNIT);
615+
long decimalRemainder = Long.remainderUnsigned(toDivide.decimal * q, SINGLE_UNIT);
616+
long val = toDivide.value * q + decimalAdjustment;
617+
if (val > this.value || (val == this.value && decimalRemainder > 0 && this.decimal < 2 * toDivide.decimal && (toDivide.decimal < 5_000 || this.decimal < 2 * (toDivide.decimal - 5_000)))) {
575618
// if we do, reduce the result by one to account for it
576619
return q - 1;
577620
}
578621
}
579622
return q;
580623
}
581624
//In this case, we're really multiplying (definitely need to account for decimal as well)
582-
if (Long.compareUnsigned(this.value, MAX_LONG_SHIFT) >= 0) {
583-
return Long.divideUnsigned(this.value, toDivide.decimal) * MAX_DECIMAL //lose some precision here, have to add modulus
584-
+ Long.divideUnsigned(Long.remainderUnsigned(this.value, toDivide.decimal) * MAX_DECIMAL, toDivide.decimal)
585-
+ (long) this.decimal * MAX_DECIMAL / toDivide.decimal;
625+
if (Long.compareUnsigned(this.value, LONG_SHIFT) > 0) {
626+
long base = Long.divideUnsigned(this.value, toDivide.decimal);// * SINGLE_UNIT;
627+
628+
long remainder = Long.remainderUnsigned(this.value, toDivide.decimal);
629+
boolean scaledRemainder = false;
630+
long b2;
631+
if (Long.compareUnsigned(remainder + (this.decimal > 0 ? 1 : 0), LONG_SHIFT) < 0) {
632+
b2 = Long.divideUnsigned(this.decimal + remainder * SINGLE_UNIT, toDivide.decimal);
633+
scaledRemainder = b2 != 0;
634+
} else {
635+
b2 = Long.divideUnsigned(this.decimal + remainder, toDivide.decimal);
636+
}
637+
long result = base + b2;
638+
if (scaledRemainder) {
639+
if (Long.compareUnsigned(base + 1, LONG_SHIFT) >= 0) {
640+
return -1;
641+
}
642+
} else if (Long.compareUnsigned(result, LONG_SHIFT) >= 0) {
643+
return -1;
644+
}
645+
long adjustedResult = scaledRemainder ? base * SINGLE_UNIT + b2 : result * SINGLE_UNIT;
646+
if (value < 0 && adjustedResult == 0) {
647+
return -1;
648+
}
649+
650+
long a2 = adjustedResult + (long) this.decimal * SINGLE_UNIT / toDivide.decimal;
651+
if (adjustedResult < 0 && a2 >= 0) {
652+
return -1;
653+
}
654+
return adjustedResult;
586655
}
587-
long d = this.value * MAX_DECIMAL;
588-
//Note: We don't care about modulus since we're returning integers
589-
return Long.divideUnsigned(d, toDivide.decimal) + ((long) this.decimal * MAX_DECIMAL / toDivide.decimal);
656+
long d = this.value * SINGLE_UNIT;
657+
return Long.divideUnsigned(d, toDivide.decimal) + Long.divideUnsigned(this.decimal + Long.remainderUnsigned(d, toDivide.decimal), toDivide.decimal);
590658
}
591659

592660
/**
@@ -1234,16 +1302,7 @@ private static long multiplyLongs(long a, long b) {
12341302
* Internal helper to multiply a long by a decimal.
12351303
*/
12361304
private static FloatingLong multiplyLongAndDecimal(long value, short decimal) {
1237-
if (value == 0 || decimal == 0) {
1238-
//Short circuit and don't create any new objects
1239-
return FloatingLong.ZERO;
1240-
}
1241-
//This can't overflow!
1242-
if (Long.compareUnsigned(value, Long.divideUnsigned(-1, SINGLE_UNIT)) > 0) {
1243-
return create(Long.divideUnsigned(value, SINGLE_UNIT) * decimal, (short) (value % SINGLE_UNIT * decimal));
1244-
}
1245-
//Instantiate directly to avoid having to clamp the decimal
1246-
return new FloatingLong(Long.divideUnsigned(value * decimal, SINGLE_UNIT), (short) (value * decimal % SINGLE_UNIT), false);
1305+
return addLongAndDecimalMultiplication(ZERO, value, decimal);
12471306
}
12481307

12491308
/**
@@ -1255,10 +1314,11 @@ private static FloatingLong addLongAndDecimalMultiplication(FloatingLong base, l
12551314
return base;
12561315
}
12571316
//This can't overflow!
1258-
if (Long.compareUnsigned(value, Long.divideUnsigned(-1, SINGLE_UNIT)) > 0) {
1259-
return base.plusEqual(Long.divideUnsigned(value, SINGLE_UNIT) * decimal, clampDecimal((short) (value % SINGLE_UNIT * decimal)));
1317+
if (Long.compareUnsigned(value, LONG_SHIFT) > 0) {
1318+
long newDecimal = Long.remainderUnsigned(value, SINGLE_UNIT) * decimal;
1319+
return base.plusEqual(Long.divideUnsigned(value, SINGLE_UNIT) * decimal + newDecimal / SINGLE_UNIT, (short) (newDecimal % SINGLE_UNIT));
12601320
}
1261-
return base.plusEqual(Long.divideUnsigned(value * decimal, SINGLE_UNIT), (short) (value * decimal % SINGLE_UNIT));
1321+
return base.plusEqual(Long.divideUnsigned(value * decimal, SINGLE_UNIT), (short) Long.remainderUnsigned(value * decimal, SINGLE_UNIT));
12621322
}
12631323

12641324
/**

0 commit comments

Comments
 (0)