Skip to content

fix: element-wise operations on transposed/swap-axes views return wrong data - #31

Merged
CodeWithKyrian merged 1 commit into
mainfrom
fix/view-strides-operations-bug
Jun 15, 2026
Merged

CodeWithKyrian merged 1 commit into
mainfrom
fix/view-strides-operations-bug

Conversation

@CodeWithKyrian

Copy link
Copy Markdown
Contributor

This PR fixes a bug where element-wise operations (clamp, log, exp, add, multiply, sqrt, etc.) on transposed or swap-axes views returned data in raw buffer order instead of the view's logical order.

Motivation and Context

When an NDArray view had strides matching an F-contiguous layout (e.g., strides [1, 3] for a transposed [3, 3] matrix), ndarray's map() fast path interpreted the raw C-ordered buffer as F-ordered data, producing wrong results. This affected all operations using element-wise mapping — clamp, log, exp, add_scalar, multiply_scalar, and 30+ math functions. The bug was discovered in production in the TransformersPHP test suite and confirmed with direct PHP-side element-by-element verification.

What's Changed

  • Renamed extract_view_* to extract_array_* and changed return type from ArrayViewD to ArrayD with internal correctness logic — C-contiguous strides use memcpy, custom strides use stride-based iteration
  • Renamed extract_array_mut_* to extract_view_mut_* to accurately reflect the mutable view return type
  • Updated all internal function signatures consuming extract_array_* results to accept &ArrayD<T> instead of ArrayViewD<T>
  • Deleted the redundant define_extract_view_as internal .to_owned() call, now handled by define_extract_array
  • Added regression tests in ShapeOpsTest covering transpose→clamp, swapaxes→clamp, and slice→transpose→clamp

Breaking Changes

None. All public PHP APIs remain unchanged. This is purely an internal Rust-layer fix

@CodeWithKyrian
CodeWithKyrian merged commit a28831c into main Jun 15, 2026
14 checks passed
@CodeWithKyrian
CodeWithKyrian deleted the fix/view-strides-operations-bug branch June 15, 2026 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant