Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BigDecimalBuilder and arithmetic operations. #9950

Merged
merged 54 commits into from
Jun 4, 2024
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
54 commits
Select commit Hold shift + click to select a range
634d12a
hack
GregoryTravis May 1, 2024
14b711b
Merge branch 'develop' into wip/gmt/8355-db-bd
GregoryTravis May 7, 2024
291a60a
Merge branch 'develop' into wip/gmt/8355-db-bd
GregoryTravis May 8, 2024
dfc854a
make a column
GregoryTravis May 8, 2024
b1c567f
add
GregoryTravis May 9, 2024
004114a
Merge branch 'develop' into wip/gmt/8355-bigdecimal-builder
GregoryTravis May 9, 2024
347e66e
Merge branch 'develop' into wip/gmt/8355-bigdecimal-builder
GregoryTravis May 10, 2024
4973d5e
no scale=0 on BD type
GregoryTravis May 10, 2024
deb15e4
Merge branch 'develop' into wip/gmt/8355-bigdecimal-builder
GregoryTravis May 13, 2024
aeb4114
a test
GregoryTravis May 13, 2024
a66676a
Merge branch 'develop' into wip/gmt/8355-bigdecimal-builder
GregoryTravis May 14, 2024
2b9a1e3
wip
GregoryTravis May 14, 2024
3514d31
Merge branch 'develop' into wip/gmt/8355-bigdecimal-builder
GregoryTravis May 14, 2024
4c0b81e
3 arithmetic ops
GregoryTravis May 14, 2024
a4fd38c
Merge branch 'develop' into wip/gmt/8355-bigdecimal-builder
GregoryTravis May 15, 2024
f940ccd
/
GregoryTravis May 15, 2024
6850751
wip
GregoryTravis May 15, 2024
f846903
Merge branch 'develop' into wip/gmt/8355-bigdecimal-builder
GregoryTravis May 16, 2024
230b8bb
BigDecimalPowerOp
GregoryTravis May 16, 2024
a2a6019
wip
GregoryTravis May 16, 2024
2ff91a5
mod test
GregoryTravis May 16, 2024
0493528
Merge branch 'develop' into wip/gmt/8355-bigdecimal-builder
GregoryTravis May 17, 2024
bb27fa4
NumericBinaryOpReturningBigDecimal
GregoryTravis May 17, 2024
fef5e4a
with scalar
GregoryTravis May 17, 2024
bdda67f
misc arithmetic tests
GregoryTravis May 17, 2024
c85bb94
Merge branch 'develop' into wip/gmt/8355-bigdecimal-builder
GregoryTravis May 20, 2024
137f975
fix integralBigDecimalToInteger
GregoryTravis May 20, 2024
3f3fde8
mixed columns
GregoryTravis May 20, 2024
f58ad25
Merge branch 'develop' into wip/gmt/8355-bigdecimal-builder
GregoryTravis May 24, 2024
5b1ef35
bigdecimal pow via double
GregoryTravis May 24, 2024
cc9230c
cleanup
GregoryTravis May 24, 2024
3cce4cb
j2e on get
GregoryTravis May 24, 2024
8d60684
arithmetic exception
GregoryTravis May 24, 2024
6c84519
mod 0
GregoryTravis May 24, 2024
1ad53b8
cleanup
GregoryTravis May 24, 2024
30a8f92
fmt
GregoryTravis May 24, 2024
ca1998a
changelog
GregoryTravis May 24, 2024
fa45de2
merge
GregoryTravis May 28, 2024
93f8f4b
check type first
GregoryTravis May 28, 2024
67d2769
merge
GregoryTravis May 29, 2024
2a87ccb
Merge branch 'develop' into wip/gmt/8355-bigdecimal-builder
GregoryTravis May 29, 2024
4a8a761
Merge branch 'develop' into wip/gmt/8355-bigdecimal-builder
GregoryTravis May 30, 2024
c2b1521
mc error message
GregoryTravis May 30, 2024
6f45e3e
Merge branch 'develop' into wip/gmt/8355-bigdecimal-builder
GregoryTravis May 30, 2024
1935981
Merge branch 'develop' into wip/gmt/8355-bigdecimal-builder
GregoryTravis May 31, 2024
45f143f
Merge branch 'develop' into wip/gmt/8355-bigdecimal-builder
GregoryTravis Jun 3, 2024
ef4ff68
add BD case to Builder.java
GregoryTravis Jun 3, 2024
517c36c
fmt
GregoryTravis Jun 3, 2024
d80d32f
changelog
GregoryTravis Jun 3, 2024
345422e
add BD case to StorageConverter.java
GregoryTravis Jun 3, 2024
e680e30
fmt
GregoryTravis Jun 3, 2024
bad8fa1
Merge branch 'develop' into wip/gmt/8355-bigdecimal-builder
GregoryTravis Jun 4, 2024
c0467d9
fix test
GregoryTravis Jun 4, 2024
0f521bb
merge
GregoryTravis Jun 4, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions distribution/lib/Standard/Table/0.0.0-dev/src/Column.enso
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import project.Value_Type.Value_Type
from project.Errors import Conversion_Failure, Floating_Point_Equality, Inexact_Type_Coercion, Invalid_Column_Names, Invalid_Value_Type, No_Index_Set_Error
from project.Internal.Column_Format import all
from project.Internal.Java_Exports import make_date_builder_adapter, make_string_builder
from project.Internal.Storage import enso_to_java, java_to_enso

polyglot java import org.enso.base.Time_Utils
polyglot java import org.enso.table.data.column.operation.cast.CastProblemAggregator
Expand Down Expand Up @@ -102,7 +103,7 @@ type Column
Invalid_Column_Names.handle_java_exception <| Polyglot_Helpers.handle_polyglot_dataflow_errors <| handle_invalid_value_type <|
java_column = Java_Problems.with_problem_aggregator Problem_Behavior.Report_Warning java_problem_aggregator->
case needs_polyglot_conversion of
True -> Java_Column.fromItems name items expected_storage_type java_problem_aggregator
True -> Java_Column.fromItems name (items.map enso_to_java) expected_storage_type java_problem_aggregator
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit worried about performance here. Have you checked Column_from_vector_* benchmarks?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added logic to check the value_type and only applying enso_to_java if it might be necessary. There's a significant slowdown in the benchmarks in the value_type=Auto cases, since it calls enso_to_java in those cases.

Before:

Benchmarking 'Column_from_vector_1000000.Integers_type_Auto' with configuration: [warmup={2 iterations, 3 seconds each}, measurement={2 iterations, 3 seconds each}]
Measurement avg time:    242.857 ms (+-2.669)
Benchmarking 'Column_from_vector_1000000.Floats_type_Auto' with configuration: [warmup={2 iterations, 3 seconds each}, measurement={2 iterations, 3 seconds each}]
Measurement avg time:    241.672 ms (+-0.935)

After:

checking value type, but not for auto, auto is worse:
Benchmarking 'Column_from_vector_1000000.Integers_type_Auto' with configuration: [warmup={2 iterations, 3 seconds each}, measurement={2 iterations, 3 seconds each}]
Measurement avg time:    426.575 ms (+-24.659)
Benchmarking 'Column_from_vector_1000000.Floats_type_Auto' with configuration: [warmup={2 iterations, 3 seconds each}, measurement={2 iterations, 3 seconds each}]
Measurement avg time:    351.581 ms (+-2.66)

@JaroslavTulach do you have any suggestions for making enso_to_java faster? Perhaps implementing it in Java?

Copy link
Member

@JaroslavTulach JaroslavTulach May 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this comment of mine be considered a suggestion? Does it have any impact on the benchmarks? It might as the number of back and forth calls is going to be different...

False -> Java_Column.fromItemsNoDateConversion name items expected_storage_type java_problem_aggregator
result = Column.Value java_column . throw_on_warning Conversion_Failure
result.catch Conversion_Failure error->
Expand Down Expand Up @@ -137,7 +138,7 @@ type Column

Arguments:
- java_column: The internal representation of the column.
private Value java_column
Value java_column

## PRIVATE
ADVANCED
Expand Down Expand Up @@ -2111,7 +2112,7 @@ type Column
@index (self-> Numeric_Input minimum=0 maximum=self.length-1)
at : Integer -> (Any | Nothing) ! Index_Out_Of_Bounds
at self (index : Integer) =
self.get index (Error.throw (Index_Out_Of_Bounds.Error index self.length))
java_to_enso <| self.get index (Error.throw (Index_Out_Of_Bounds.Error index self.length))

## GROUP Standard.Base.Selections
ICON select_row
Expand Down Expand Up @@ -2158,7 +2159,7 @@ type Column

example_to_vector = Examples.integer_column.to_vector
to_vector : Vector
to_vector self = Vector.from_polyglot_array self.java_column.getStorage.toList
to_vector self = Vector.from_polyglot_array self.java_column.getStorage.toList . map java_to_enso

## GROUP Standard.Base.Metadata
ICON metadata
Expand Down Expand Up @@ -2458,7 +2459,7 @@ run_vectorized_binary_op column name operand new_name=Nothing fallback_fn=Nothin
_ ->
s1 = column.java_column.getStorage
rs = Polyglot_Helpers.handle_polyglot_dataflow_errors <|
s1.vectorizedOrFallbackBinaryMap name problem_builder fallback_fn operand skip_nulls storage_type
s1.vectorizedOrFallbackBinaryMap name problem_builder fallback_fn (enso_to_java operand) skip_nulls storage_type
Column.Value (Java_Column.new effective_new_name rs)

## PRIVATE
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,11 @@ import project.Value_Type.Bits
import project.Value_Type.Value_Type
from project.Errors import Inexact_Type_Coercion

polyglot java import java.math.BigDecimal

polyglot java import org.enso.table.data.column.builder.Builder as Java_Builder
polyglot java import org.enso.table.data.column.storage.type.AnyObjectType
polyglot java import org.enso.table.data.column.storage.type.BigDecimalType
polyglot java import org.enso.table.data.column.storage.type.BigIntegerType
polyglot java import org.enso.table.data.column.storage.type.Bits as Java_Bits
polyglot java import org.enso.table.data.column.storage.type.BooleanType
Expand Down Expand Up @@ -40,6 +43,7 @@ to_value_type storage_type = case storage_type of
_ : DateType -> Value_Type.Date
_ : DateTimeType -> Value_Type.Date_Time with_timezone=True
_ : TimeOfDayType -> Value_Type.Time
_ : BigDecimalType -> Value_Type.Decimal
_ : BigIntegerType -> Value_Type.Decimal scale=0
_ : AnyObjectType -> Value_Type.Mixed

Expand All @@ -64,11 +68,18 @@ closest_storage_type value_type = case value_type of
Value_Type.Mixed -> AnyObjectType.INSTANCE
Value_Type.Decimal _ scale ->
is_integer = scale.is_nothing || scale <= 0
if is_integer then BigIntegerType.INSTANCE else
Error.throw (Illegal_Argument.Error "Columns of type "+value_type.to_display_text+" are currently not supported in the in-memory backend - only Decimal of integer type (scale <= 0) is supported. You may cast the column to Float first (lossy conversion).")
if is_integer then BigIntegerType.INSTANCE else BigDecimalType.INSTANCE
_ ->
Error.throw (Illegal_Argument.Error "Columns of type "+value_type.to_display_text+" are currently not supported in the in-memory backend.")

enso_to_java x = case x of
Decimal.Value big_decimal -> big_decimal
_ -> x

java_to_enso x = case x of
_ : BigDecimal -> Decimal.Value x
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

java_to_enso is a conversion function that wraps java.math.BigDecimal into Enso's Decimal by calling the Value constructor. That's correct wrapping. What kind of problem is associated with it? Are you searching for alternatives?

You can call the factory in Java, if you want. If you create a method:

public static Object factory(Function<BigDecimal,Object> f) {
  return f.apply(BigDecimal.valueOf(1));
}

and call it from Enso as

IO.println <| factory Decimal.Value

then you should get Decimal.Value 1. Other than this, I am not sure what to do/change.

_ -> x

## PRIVATE
Converts a value type to an in-memory storage type, possibly approximating it
to the closest supported type.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ most_specific_value_type : Any -> Boolean -> Value_Type
most_specific_value_type value use_smallest=False =
case value of
_ : Float -> Value_Type.Float Bits.Bits_64
_ : Decimal -> Value_Type.Decimal
_ : Boolean -> Value_Type.Boolean
_ : Date -> Value_Type.Date
_ : Time_Of_Day -> Value_Type.Time
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@
* want to be consistent with Enso's equality semantics where 2 == 2.0.
*/
public class NumericConverter {
public static BigInteger INTEGER_MIN_VALUE_BIG_INTEGER = BigInteger.valueOf(Integer.MIN_VALUE);
public static BigInteger INTEGER_MAX_VALUE_BIG_INTEGER = BigInteger.valueOf(Integer.MAX_VALUE);
public static BigDecimal INTEGER_MIN_VALUE_BIG_DECIMAL = BigDecimal.valueOf(Integer.MIN_VALUE);
public static BigDecimal INTEGER_MAX_VALUE_BIG_DECIMAL = BigDecimal.valueOf(Integer.MAX_VALUE);

/**
* Coerces a number (possibly an integer) to a Double.
*
Expand Down Expand Up @@ -59,14 +64,32 @@ public static BigInteger coerceToBigInteger(Object o) {
}
}

/**
* Coerces a number to a BigDecimal.
*
* <p>Will throw an exception if the object is not a number.
*/
public static BigDecimal coerceToBigDecimal(Object o) {
return switch (o) {
case Double x -> BigDecimal.valueOf(x);
case BigDecimal x -> x;
case Float x -> BigDecimal.valueOf(x);
case BigInteger x -> new BigDecimal(x);
case Long x -> BigDecimal.valueOf(x);
case Integer x -> BigDecimal.valueOf(x);
case Short x -> BigDecimal.valueOf(x);
case Byte x -> BigDecimal.valueOf(x);
default -> throw new UnsupportedOperationException("Cannot coerce " + o + " to a BigDecimal.");
};
}

/** Returns true if the object is any supported number. */
public static boolean isCoercibleToDouble(Object o) {
return isFloatLike(o)|| isCoercibleToLong(o) || o instanceof BigInteger;
}

public static boolean isFloatLike(Object o) {
return o instanceof Double
|| o instanceof BigDecimal
|| o instanceof Float;
}

Expand Down Expand Up @@ -102,6 +125,77 @@ public static Double tryConvertingToDouble(Object o) {
};
}

/**
* Tries converting the value to an Integer.
*
* <p>Decimal number types are accepted, only if their fractional part is 0. It will return null
* if the object represented a non-integer value. Integer values must fit within the Integer range.
*/
public static Integer tryConvertingToInteger(Object o) {
return switch (o) {
case Long x -> fitsInInt(x) ? x.intValue() : null;
case Integer x -> x;
case Short x -> (int) x;
case Byte x -> (int) x;
case Double x -> integralDoubleToInteger(x);
case Float x -> integralDoubleToInteger(x);
case BigDecimal x -> integralBigDecimalToInteger(x);
case BigInteger x -> integralBigIntegerToInteger(x);
case null, default -> null;
};
}

private static boolean fitsInInt(long x) {
return x >= Integer.MIN_VALUE && x <= Integer.MAX_VALUE;
}

private static boolean fitsInInt(BigInteger x) {
return x.compareTo(INTEGER_MIN_VALUE_BIG_INTEGER) >= 0 && x.compareTo(INTEGER_MAX_VALUE_BIG_INTEGER) <= 0;
}

private static boolean fitsInInt(BigDecimal x) {
return x.compareTo(INTEGER_MIN_VALUE_BIG_DECIMAL) >= 0 && x.compareTo(INTEGER_MAX_VALUE_BIG_DECIMAL) <= 0;
}

/**
* Converts a double to an Integer, if the value is integral and fits in the
* Integer range. Otherwise, returns null;
*/
private static Integer integralDoubleToInteger(double x) {
if (x % 1.0 == 0.0) {
long l = (long) x;
if (fitsInInt(l)) {
return (int) l;
}
}
return null;
}

/**
* Converts a BigInteger to an Integer, if the value fits in the Integer
* range. Otherwise, returns null;
*/
private static Integer integralBigIntegerToInteger(BigInteger x) {
if (fitsInInt(x)) {
return x.intValueExact();
} else {
return null;
}
}

/**
* Converts a BigDecmal to an Integer, if the value is integral and fits in
* the Integer range. Otherwise, returns null;
*/
private static Integer integralBigDecimalToInteger(BigDecimal x) {
if (x.remainder(BigDecimal.ZERO).equals(BigDecimal.ZERO)) {
if (fitsInInt(x)) {
return x.intValueExact();
}
}
return null;
}

/**
* Tries converting the value to a Long.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ public static Object convertPolyglotValue(Value item) {
if (item.isException()) {
throw new WrappedDataflowError(item);
}

var ret = item.as(Object.class);
if (ret instanceof BigInteger && item.fitsInLong()) {
return item.asLong();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package org.enso.table.data.column.builder;

import java.math.BigDecimal;

import org.enso.table.data.column.storage.Storage;
import org.enso.table.data.column.storage.numeric.BigDecimalStorage;
import org.enso.table.data.column.storage.type.BigDecimalType;
import org.enso.table.data.column.storage.type.StorageType;
import org.enso.table.error.ValueTypeMismatchException;

/** A builder for BigDecimal columns. */
public class BigDecimalBuilder extends TypedBuilderImpl<BigDecimal> {
@Override
protected BigDecimal[] newArray(int size) {
return new BigDecimal[size];
}

public BigDecimalBuilder(int size) {
super(size);
}

@Override
public StorageType getType() {
return BigDecimalType.INSTANCE;
}

@Override
public void appendNoGrow(Object o) {
try {
data[currentSize++] = (BigDecimal) o;
} catch (ClassCastException e) {
throw new ValueTypeMismatchException(getType(), o);
}
}

@Override
public void append(Object o) {
appendNoGrow(o);
}

@Override
public boolean accepts(Object o) {
return o instanceof BigDecimal;
}

@Override
protected Storage<BigDecimal> doSeal() {
return new BigDecimalStorage(data, currentSize);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import org.enso.table.data.column.storage.type.StorageType;
import org.enso.table.error.ValueTypeMismatchException;

/** A builder for string columns. */
/** A builder for LocalDate columns. */
public class DateBuilder extends TypedBuilderImpl<LocalDate> {
@Override
protected LocalDate[] newArray(int size) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import org.enso.table.data.column.storage.type.StorageType;
import org.enso.table.error.ValueTypeMismatchException;

/** A builder for string columns. */
/** A builder for ZonedDateTime columns. */
public class DateTimeBuilder extends TypedBuilderImpl<ZonedDateTime> {
@Override
protected ZonedDateTime[] newArray(int size) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.enso.table.data.column.builder;

import java.math.BigDecimal;
import java.math.BigInteger;
import java.time.LocalDate;
import java.time.LocalTime;
Expand Down Expand Up @@ -115,6 +116,8 @@ private void initBuilderFor(Object o) {
currentBuilder = new StringBuilder(initialCapacity, TextType.VARIABLE_LENGTH);
} else if (o instanceof BigInteger) {
currentBuilder = new BigIntegerBuilder(initialCapacity, problemAggregator);
} else if (o instanceof BigDecimal) {
currentBuilder = new BigDecimalBuilder(initialCapacity);
} else if (o instanceof LocalDate) {
currentBuilder = new DateBuilder(initialCapacity);
} else if (o instanceof LocalTime) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import org.enso.table.data.column.storage.type.TimeOfDayType;
import org.enso.table.error.ValueTypeMismatchException;

/** A builder for string columns. */
/** A builder for LocalTime columns. */
public class TimeOfDayBuilder extends TypedBuilderImpl<LocalTime> {
@Override
protected LocalTime[] newArray(int size) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package org.enso.table.data.column.operation.map.numeric.arithmetic;

import java.math.BigDecimal;
import java.math.BigInteger;
import org.enso.table.data.column.operation.map.MapOperationProblemAggregator;
import org.enso.table.data.column.storage.Storage;
import org.enso.table.data.column.storage.numeric.BigDecimalStorage;
import org.enso.table.data.column.storage.type.IntegerType;

public class AddOp<T extends Number, I extends Storage<? super T>>
Expand Down Expand Up @@ -32,4 +34,10 @@ public BigInteger doBigInteger(
BigInteger a, BigInteger b, int ix, MapOperationProblemAggregator problemAggregator) {
return a.add(b);
}

@Override
public BigDecimal doBigDecimal(
BigDecimal a, BigDecimal b, int ix, MapOperationProblemAggregator problemAggregator) {
return a.add(b);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package org.enso.table.data.column.operation.map.numeric.arithmetic;

import java.math.BigDecimal;

import org.enso.table.data.column.operation.map.MapOperationProblemAggregator;
import org.enso.table.data.column.storage.Storage;

public class BigDecimalDivideOp<T extends Number, I extends Storage<? super T>>
extends NumericBinaryOpReturningBigDecimal<T, I> {
public BigDecimalDivideOp() {
super(Storage.Maps.DIV);
}

@Override
public BigDecimal doBigDecimal(
BigDecimal a, BigDecimal b, int ix, MapOperationProblemAggregator problemAggregator) {
return a.divide(b);
}
}
Loading
Loading