Skip to content

Cast datatypes at runtime - #39

Open
apphp wants to merge 2 commits into
RubixML:mainfrom
apphp:branch-38
Open

apphp wants to merge 2 commits into
RubixML:mainfrom
apphp:branch-38

Conversation

@apphp

@apphp apphp commented Oct 3, 2026 •

Copy link
Copy Markdown

Summary

Adds NDArray::cast(string $dtype) and its alias NDArray::toDataType(string $dtype) to change an array's datatype at runtime.

  • Converts the array in place, like setDataType(): the same PHP object (and every reference to it) gets the new dtype, so no new NDArray needs to be created.
  • Returns $this, so calls can be chained: $x->cast('float16')->toArray().
  • The device (CPU/GPU) is kept. Casting to the current dtype does nothing. An unknown dtype throws Error and leaves the array unchanged.
  • To keep the original, cast a copy: NumPower::copy($x)->cast('int8').
$ndarray->cast('float16');
$ndarray->toDataType('float16');

Ticket: #38

Testing

  • make test passes with USE_ZEND_ALLOC=1 and USE_ZEND_ALLOC=0 (273 passed, 0 failed, 129 skipped — GPU tests) on PHP 8.4; tests/types also on PHP 8.1 and 8.5.
  • tests/types run under Valgrind 3.19 (run-tests.php -m with VALGRIND_OPTS="--leak-check=full --errors-for-leak-kinds=definite"): 31 passed, 0 leaked, 0 memory errors.
    Sanity check: with NDArray_FREE(prev) removed from the cast path, the same run reports LEAK on 031-cast.phpt.
  • Not tested on GPU (no CUDA device available).

Submission Checklist:

  • Have you followed the guidelines in our Contributing document? (no CONTRIBUTING.md in the repo; followed existing code/test style)
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Does your submission pass tests with ZEND_ALLOC enabled? export USE_ZEND_ALLOC=1 && make test
  • Does your submission pass tests with ZEND_ALLOC disabled? export USE_ZEND_ALLOC=0 && make test (also tests/types under Valgrind: 0 leaks, 0 memory errors — see Testing)

Change to methods and operations

  • Have you verified that your change does not break backwards compatibility? (new methods only; setDataType() now also rejects dtype names containing a NUL byte)
  • Have you updated the operation(s) tests for your changes, as applicable? (tests/types/031–035)
  • Optional: Have your changes also been tested on the GPU? (no GPU available — 032-cast-gpu.phpt needs a reviewer with CUDA)
  • Optional: Have your changes also been tested on the GPU with NDARRAY_VCHECK enabled?

Changes to Core Components:

N/A — no changes to buffer.c, gpu_alloc.c, ndarray.c, iterators.c or their headers. Only numpower.c (method bindings) was modified.

@apphp apphp self-assigned this Oct 3, 2026
@apphp apphp added the enhancement New feature or request label Oct 3, 2026
@apphp
apphp requested a review from a team October 3, 2026 21:12

@andrewdalpino andrewdalpino left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So we have setDataType() that casts in-place but does not return this, cast that casts in place but does return this, and toDataType() which is an alias of cast? Is that right @apphp?

Comment thread tests/types/031-cast.phpt
--TEST--
NDArray::cast() / toDataType() change the dtype in place and return $this
--FILE--
<?php

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Totally unrelated, but what are your thoughts on either

A. Converting tests that only test the public API to PHP Unit
B. Only testing the extension through the public API, i.e. subsystems are tested indirectly, and dropping all these .phpt style tests

?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants