Skip to content

Commit

Permalink
Remove storage from offset (#12130)
Browse files Browse the repository at this point in the history
* Remove storage from offset

* fmt

* Doc fix
  • Loading branch information
AdRiley authored Jan 24, 2025
1 parent 092d7c0 commit 6592bae
Show file tree
Hide file tree
Showing 11 changed files with 30 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,15 @@ table_offset_impl table:Table (columns:(Vector | Text | Integer | Regex)=[]) n:I
new_names = resolved_columns.map c->
if set_mode!=Set_Mode.Add then c.name else
Column_Naming_Helper.in_memory.function_name "offset" [c, n, fillWith]
new_storages = Java_Problems.with_problem_aggregator on_problems java_problem_aggregator->
new_columns_java = Java_Problems.with_problem_aggregator on_problems java_problem_aggregator->
Java_Offset.offset resolved_java_columns n fillWith_java grouping_java_columns ordering_java_columns directions java_problem_aggregator
new_columns = new_names.zip new_storages.to_vector new_name-> new_storage -> Column.from_storage new_name new_storage
new_columns.fold table t-> c->
t.set c c.name set_mode
new_columns = new_columns_java.map Column.from_java_column
new_columns.zip new_names . fold table t-> pair->
t.set pair.first pair.second set_mode

column_offset_impl column:Column n:Integer=-1 fillWith:Fill_With=..Nothing =
java_column = column.java_column
fillWith_java = fillWith.to_java
new_storage = Java_Offset.offset_single_column java_column n fillWith_java
new_column = Column.from_java_column <| Java_Offset.offset_single_column java_column n fillWith_java
new_name = Column_Naming_Helper.in_memory.function_name "offset" [column, n, fillWith]
Column.from_storage new_name new_storage
new_column.rename new_name
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ public BoolStorage applyMask(OrderMask mask) {
BitSet newVals = new BitSet();
for (int i = 0; i < mask.length(); i++) {
int position = mask.get(i);
if (position == Storage.NOT_FOUND_INDEX || isNothing.get(position)) {
if (position == OrderMask.NOT_FOUND_INDEX || isNothing.get(position)) {
newNa.set(i);
} else if (values.get(position)) {
newVals.set(i);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ public SpecializedStorage<T> applyMask(OrderMask mask) {
T[] newData = newUnderlyingArray(mask.length());
for (int i = 0; i < mask.length(); i++) {
int position = mask.get(i);
newData[i] = position == Storage.NOT_FOUND_INDEX ? null : data[position];
newData[i] = position == OrderMask.NOT_FOUND_INDEX ? null : data[position];
context.safepoint();
}
return newInstance(newData, newData.length);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,6 @@

/** An abstract representation of a data column. */
public abstract class Storage<T> implements ColumnStorage {
/** A constant representing the index of a missing value in a column. */
public static final int NOT_FOUND_INDEX = -1;

/**
* @return the number of elements in this column (including NAs)
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public Storage<Long> applyMask(OrderMask mask) {
Context context = Context.getCurrent();
for (int i = 0; i < mask.length(); i++) {
int position = mask.get(i);
if (position == Storage.NOT_FOUND_INDEX) {
if (position == OrderMask.NOT_FOUND_INDEX) {
newIsNothing.set(i);
} else {
newData[i] = getItem(position);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ public Storage<Long> applyMask(OrderMask mask) {
Context context = Context.getCurrent();
for (int i = 0; i < mask.length(); i++) {
int position = mask.get(i);
if (position == Storage.NOT_FOUND_INDEX) {
if (position == OrderMask.NOT_FOUND_INDEX) {
newIsNothing.set(i);
} else {
Long item = computeItem(position);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ public Storage<Double> applyMask(OrderMask mask) {
Context context = Context.getCurrent();
for (int i = 0; i < mask.length(); i++) {
int position = mask.get(i);
if (position == Storage.NOT_FOUND_INDEX || isNothing.get(position)) {
if (position == OrderMask.NOT_FOUND_INDEX || isNothing.get(position)) {
newIsNothing.set(i);
} else {
newData[i] = data[position];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ public Storage<Long> applyMask(OrderMask mask) {
Context context = Context.getCurrent();
for (int i = 0; i < mask.length(); i++) {
int position = mask.get(i);
if (position == Storage.NOT_FOUND_INDEX || isNothing.get(position)) {
if (position == OrderMask.NOT_FOUND_INDEX || isNothing.get(position)) {
newIsNothing.set(i);
} else {
newData[i] = data[position];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,17 @@

/** Describes a storage reordering operator. */
public interface OrderMask {
/** A constant representing the index of a missing value in a column. */
public static final int NOT_FOUND_INDEX = -1;

int length();

/**
* Describes the reordering that should happen on the applying storage at the index.
*
* <p>The resulting storage should contain the {@code positions[i]}-th element of the original
* storage at the {@code idx}-th position. It may return {@link
* org.enso.table.data.storage.Storage.NOT_FOUND_INDEX}, in which case a missing value should be
* inserted at this position.
* storage at the {@code idx}-th position. It may return {@link NOT_FOUND_INDEX}, in which case a
* missing value should be inserted at this position.
*
* <p>Indices may appear zero or multiple times in the mask - meaning rows that will be gone or
* duplicated.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,12 +104,12 @@ private Table join() {
// Find corresponding row in the lookup table
int lookupRow = findLookupRow(i);

assert allowUnmatchedRows || lookupRow != Storage.NOT_FOUND_INDEX;
assert allowUnmatchedRows || lookupRow != OrderMask.NOT_FOUND_INDEX;

// Merge columns replacing old values
for (LookupOutputColumn.MergeColumns mergeColumns : columnsToMerge) {
Object itemToAdd;
if (lookupRow != Storage.NOT_FOUND_INDEX) {
if (lookupRow != OrderMask.NOT_FOUND_INDEX) {
itemToAdd = mergeColumns.lookupReplacement.getItemBoxed(lookupRow);
} else {
itemToAdd = mergeColumns.original.getItemBoxed(i);
Expand All @@ -132,7 +132,7 @@ private int findLookupRow(int baseRowIx) {
List<Integer> lookupRowIndices = lookupIndex.get(key);
if (lookupRowIndices == null) {
if (allowUnmatchedRows) {
return Storage.NOT_FOUND_INDEX;
return OrderMask.NOT_FOUND_INDEX;
} else {
List<Object> exampleKeyValues =
IntStream.range(0, keyColumnNames.size()).mapToObj(key::get).toList();
Expand Down
22 changes: 10 additions & 12 deletions std-bits/table/src/main/java/org/enso/table/operations/Offset.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,20 @@
import java.util.LinkedList;
import java.util.Queue;
import java.util.stream.IntStream;
import org.enso.table.data.column.storage.Storage;
import org.enso.table.data.mask.OrderMask;
import org.enso.table.data.table.Column;
import org.enso.table.problems.ProblemAggregator;

public class Offset {
public static Storage<?>[] offset(
public static Column[] offset(
Column[] sourceColumns,
int n,
FillWith fillWith,
Column[] groupingColumns,
Column[] orderingColumns,
int[] directions,
ProblemAggregator problemAggregator) {
if (n == 0 || sourceColumns.length == 0)
return Arrays.stream(sourceColumns).map(c -> c.getStorage()).toArray(Storage<?>[]::new);
if (n == 0 || sourceColumns.length == 0) return sourceColumns;
var rowOrderMask =
groupingColumns.length == 0 && orderingColumns.length == 0
? calculate_ungrouped_unordered_mask(sourceColumns[0].getSize(), n, fillWith)
Expand All @@ -32,14 +30,14 @@ public static Storage<?>[] offset(
directions,
problemAggregator);
return Arrays.stream(sourceColumns)
.map(c -> c.getStorage().applyMask(OrderMask.fromArray(rowOrderMask)))
.toArray(Storage<?>[]::new);
.map(c -> c.applyMask(OrderMask.fromArray(rowOrderMask)))
.toArray(Column[]::new);
}

public static Storage<?> offset_single_column(Column sourceColumn, int n, FillWith fillWith) {
if (n == 0) return sourceColumn.getStorage();
public static Column offset_single_column(Column sourceColumn, int n, FillWith fillWith) {
if (n == 0) return sourceColumn;
var rowOrderMask = calculate_ungrouped_unordered_mask(sourceColumn.getSize(), n, fillWith);
return sourceColumn.getStorage().applyMask(OrderMask.fromArray(rowOrderMask));
return sourceColumn.applyMask(OrderMask.fromArray(rowOrderMask));
}

private static int[] calculate_ungrouped_unordered_mask(int numRows, int n, FillWith fillWith) {
Expand All @@ -52,13 +50,13 @@ private static int calculate_row_offset(int rowIndex, int n, FillWith fillWith,
int result = rowIndex + n;
if (result < 0) {
return switch (fillWith) {
case NOTHING -> Storage.NOT_FOUND_INDEX;
case NOTHING -> OrderMask.NOT_FOUND_INDEX;
case CLOSEST_VALUE -> 0;
case WRAP_AROUND -> (result % numRows) == 0 ? 0 : (result % numRows) + numRows;
};
} else if (result >= numRows) {
return switch (fillWith) {
case NOTHING -> Storage.NOT_FOUND_INDEX;
case NOTHING -> OrderMask.NOT_FOUND_INDEX;
case CLOSEST_VALUE -> numRows - 1;
case WRAP_AROUND -> result % numRows;
};
Expand Down Expand Up @@ -162,7 +160,7 @@ public void finalise() {

int getFillValue() {
return switch (fillWith) {
case NOTHING -> Storage.NOT_FOUND_INDEX;
case NOTHING -> OrderMask.NOT_FOUND_INDEX;
case CLOSEST_VALUE -> closestPos;
case WRAP_AROUND -> n < 0 ? rolling_queue.poll() : fill_queue.poll();
};
Expand Down

0 comments on commit 6592bae

Please sign in to comment.