Skip to content

Commit 430093e

Browse files
committed
split up functions in reshape to make more readable
1 parent 8a828c7 commit 430093e

1 file changed

Lines changed: 112 additions & 26 deletions

File tree

src/main/java/dev/zarr/zarrjava/v3/codec/core/ReshapeCodec.java

Lines changed: 112 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -131,15 +131,47 @@ public ArrayMetadata.CoreArrayMetadata resolveArrayMetadata() throws ZarrExcepti
131131

132132
/**
133133
* Resolves the concrete output shape {@code B_shape} from the {@code shape} configuration and the
134-
* given input shape {@code A_shape}, validating all invariants mandated by the specification.
134+
* given input shape {@code A_shape}, validating all invariants mandated by the specification.
135135
*/
136136
int[] resolveOutputShape(int[] inputShape) throws ZarrException {
137-
int ndim = inputShape.length;
138-
long inputTotal = 1L;
139-
for (int size : inputShape) {
140-
inputTotal *= size;
137+
long inputTotal = product(inputShape);
138+
139+
// read the 'shape' config into a concrete (still -1-placeholder) output shape.
140+
ParsedShape parsed = parseConfiguredShape(inputShape);
141+
142+
// reject configs that reorder input dimensions (use transport for them).
143+
checkNoReordering(parsed.flatInputDims);
144+
145+
// fill in the single -1 entry, if present, so the element count matches.
146+
resolveInferredDimension(parsed.outputShape, parsed.minusOnePos, inputTotal);
147+
148+
// The fundamental reshape law, prod(output) == prod(input).
149+
checkElementCountPreserved(parsed.outputShape, inputTotal);
150+
151+
// Step 5: every merge entry must combine adjacent input dims sitting in the same flat position.
152+
checkMergesAligned(parsed.outputShape, parsed.inputDimsPerOutput, inputShape);
153+
154+
// Step 6: narrow long -> int for the array API.
155+
return toIntShape(parsed.outputShape);
156+
}
157+
158+
/**
159+
* Product of all entries of {@code shape}, computed in {@code long} to avoid overflow.
160+
*/
161+
private static long product(int[] shape) {
162+
long total = 1L;
163+
for (int size : shape) {
164+
total *= size;
141165
}
166+
return total;
167+
}
142168

169+
/**
170+
* Step 1. Reads {@code configuration.shape} into the intermediate {@link ParsedShape}: each entry
171+
* becomes a literal size, a {@code -1} placeholder, or the product of the merged input dimensions.
172+
*/
173+
private ParsedShape parseConfiguredShape(int[] inputShape) throws ZarrException {
174+
int ndim = inputShape.length;
143175
int n = configuration.shape.length;
144176
if (n == 0) {
145177
throw new ZarrException("reshape codec: 'shape' must not be empty.");
@@ -183,34 +215,49 @@ int[] resolveOutputShape(int[] inputShape) throws ZarrException {
183215
outputShape[i] = product;
184216
}
185217
}
218+
return new ParsedShape(outputShape, inputDimsPerOutput, minusOnePos, flatInputDims);
219+
}
186220

187-
// The flattened list of input dimensions must be strictly monotonically increasing. This rules
188-
// out configurations that would (incorrectly) suggest a transpose.
221+
/**
222+
* Step 2. The flattened list of referenced input dimensions must be strictly increasing. This rules
223+
* out configurations that would (incorrectly) suggest a transpose.
224+
*/
225+
private static void checkNoReordering(List<Integer> flatInputDims) throws ZarrException {
189226
for (int j = 1; j < flatInputDims.size(); j++) {
190227
if (flatInputDims.get(j) <= flatInputDims.get(j - 1)) {
191228
throw new ZarrException(
192229
"reshape codec: the flattened list of input dimensions must be strictly increasing, but got "
193230
+ flatInputDims + ".");
194231
}
195232
}
233+
}
196234

197-
// Resolve the automatic dimension (-1).
198-
if (minusOnePos != -1) {
199-
long known = 1L;
200-
for (int i = 0; i < n; i++) {
201-
if (i != minusOnePos) {
202-
known *= outputShape[i];
203-
}
204-
}
205-
if (known == 0 || inputTotal % known != 0) {
206-
throw new ZarrException(
207-
"reshape codec: cannot infer the -1 dimension because prod(output shape) would not equal "
208-
+ "prod(input shape) (" + inputTotal + ").");
235+
/**
236+
* Step 3. Resolves the automatic dimension ({@code -1}) so that {@code prod(output) == prod(input)}.
237+
*/
238+
private static void resolveInferredDimension(long[] outputShape, int minusOnePos, long inputTotal)
239+
throws ZarrException {
240+
if (minusOnePos == -1) {
241+
return;
242+
}
243+
long known = 1L;
244+
for (int i = 0; i < outputShape.length; i++) {
245+
if (i != minusOnePos) {
246+
known *= outputShape[i];
209247
}
210-
outputShape[minusOnePos] = inputTotal / known;
211248
}
249+
if (known == 0 || inputTotal % known != 0) {
250+
throw new ZarrException(
251+
"reshape codec: cannot infer the -1 dimension because prod(output shape) would not equal "
252+
+ "prod(input shape) (" + inputTotal + ").");
253+
}
254+
outputShape[minusOnePos] = inputTotal / known;
255+
}
212256

213-
// Invariant: prod(B_shape) == prod(A_shape).
257+
/**
258+
* Step 4. Invariant: {@code prod(B_shape) == prod(A_shape)}. A reshape never changes the element count.
259+
*/
260+
private static void checkElementCountPreserved(long[] outputShape, long inputTotal) throws ZarrException {
214261
long outputTotal = 1L;
215262
for (long size : outputShape) {
216263
outputTotal *= size;
@@ -220,9 +267,18 @@ int[] resolveOutputShape(int[] inputShape) throws ZarrException {
220267
"reshape codec: prod(output shape)=" + outputTotal + " does not equal prod(input shape)="
221268
+ inputTotal + ".");
222269
}
270+
}
223271

224-
// When an output dimension is specified by input_dims, verify that the coordinates along those
225-
// input dimensions actually correspond to the raveled index along that output dimension.
272+
/**
273+
* Step 5. For each output dimension specified by input dimensions, verify that the merged input
274+
* dimensions actually correspond to the raveled index along that output dimension: the flat range
275+
* before and after the merged block must match the flat range before and after the output dimension.
276+
* This is a pure check &mdash; it never modifies {@code outputShape}.
277+
*/
278+
private static void checkMergesAligned(long[] outputShape, int[][] inputDimsPerOutput, int[] inputShape)
279+
throws ZarrException {
280+
int n = outputShape.length;
281+
int ndim = inputShape.length;
226282
for (int i = 0; i < n; i++) {
227283
int[] inputDims = inputDimsPerOutput[i];
228284
if (inputDims == null || inputDims.length == 0) {
@@ -252,9 +308,15 @@ int[] resolveOutputShape(int[] inputShape) throws ZarrException {
252308
+ " vs " + inputSuffix + ").");
253309
}
254310
}
311+
}
255312

256-
int[] result = new int[n];
257-
for (int i = 0; i < n; i++) {
313+
/**
314+
* Step 6. Narrows the resolved {@code long} shape to {@code int[]} (the array API type), rejecting
315+
* any dimension that would not fit.
316+
*/
317+
private static int[] toIntShape(long[] outputShape) throws ZarrException {
318+
int[] result = new int[outputShape.length];
319+
for (int i = 0; i < outputShape.length; i++) {
258320
if (outputShape[i] > Integer.MAX_VALUE) {
259321
throw new ZarrException("reshape codec: output dimension " + i + " exceeds Integer.MAX_VALUE.");
260322
}
@@ -263,6 +325,29 @@ int[] resolveOutputShape(int[] inputShape) throws ZarrException {
263325
return result;
264326
}
265327

328+
/**
329+
* Intermediate result of {@link #parseConfiguredShape}: the (possibly still {@code -1}-containing)
330+
* output shape plus the bookkeeping the later validation steps need.
331+
*/
332+
private static final class ParsedShape {
333+
/** Output shape; the {@code -1} entry (if any) is still a placeholder until step 3. */
334+
final long[] outputShape;
335+
/** Per output dimension, the merged input dims (or {@code null} for a literal/{@code -1} entry). */
336+
final int[][] inputDimsPerOutput;
337+
/** Index of the single {@code -1} entry, or {@code -1} if there is none. */
338+
final int minusOnePos;
339+
/** All referenced input dimensions, flattened in reading order (used by the no-reorder check). */
340+
final List<Integer> flatInputDims;
341+
342+
ParsedShape(long[] outputShape, int[][] inputDimsPerOutput, int minusOnePos,
343+
List<Integer> flatInputDims) {
344+
this.outputShape = outputShape;
345+
this.inputDimsPerOutput = inputDimsPerOutput;
346+
this.minusOnePos = minusOnePos;
347+
this.flatInputDims = flatInputDims;
348+
}
349+
}
350+
266351
private static int[] asInputDims(Object element) throws ZarrException {
267352
if (element instanceof int[]) {
268353
return (int[]) element;
@@ -291,7 +376,8 @@ private static int asInteger(Object element) throws ZarrException {
291376
return ((Number) element).intValue();
292377
}
293378
throw new ZarrException(
294-
"reshape codec: 'shape' entries must be integers or arrays of integers, but got " + element + ".");
379+
"reshape codec: 'shape' entries must be integers or arrays of integers, but got " + element + "."
380+
);
295381
}
296382

297383
public static final class Configuration {

0 commit comments

Comments
 (0)