Skip to content

Add array type parameter to DefaultArrayInterface #44

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

Merged
merged 3 commits into from
Jun 12, 2025
Merged

Conversation

mtfishman
Copy link
Member

@mtfishman mtfishman commented Jun 12, 2025

EDIT: This description is a bit outdated since it is all handled by just adding a new type parameter to DefaultArrayInterface but the idea is still relevant:

This adds another AbstractArrayInterface subtype called ArrayInterface. It is analogous to Base.Broadcast.ArrayStyle. Specifically, it can act as the array style for any array type by storing the array type as a type parameter.

In the current PR, when you call interface(A::AbstractArray), it has the following behavior:

  1. It unwraps any array type wrappers from typeof(A).
  2. If the unwrapped type is Array or an abstract array type like AbstractArray, it constructs a DefaultArrayInterface.
  3. Otherwise, it creates an ArrayStyle with the unwrapped array type of A as a type parameter, though to canonicalize the type it strips off all type parameters.

Then, similar(::ArrayStyle, T::Type, ax::Tuple) sets the element type of the stored array type (using TypeParameterAccessors.set_eltype) and tries to construct the array with similar(A{T}, ax). This of course can go wrong, in which case that array type should define a custom interface. However, this works for a lot of cases automatically. The motivation is ITensor/BlockSparseArrays.jl#138, where I want the interface system to automatically work for GPU array types without having to make a custom interface for each one, this PR handles that case. We may want custom GPU array interfaces anyway at some point, for example to store information about the device, storage mode like unified memory, etc., but for now I think it is nice to have something that "just works".

Copy link

codecov bot commented Jun 12, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.89%. Comparing base (bb0a449) to head (c53cf32).

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #44      +/-   ##
==========================================
+ Coverage   72.23%   73.89%   +1.65%     
==========================================
  Files          11       11              
  Lines         371      383      +12     
==========================================
+ Hits          268      283      +15     
+ Misses        103      100       -3     
Flag Coverage Δ
docs 27.03% <0.00%> (-0.88%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mtfishman mtfishman changed the title Add ArrayInterface [WIP] Add ArrayInterface Jun 12, 2025
@mtfishman mtfishman marked this pull request as draft June 12, 2025 15:22
@mtfishman mtfishman marked this pull request as ready for review June 12, 2025 15:44
@mtfishman mtfishman changed the title [WIP] Add ArrayInterface Add ArrayInterface Jun 12, 2025
@mtfishman mtfishman requested a review from lkdvos June 12, 2025 15:48
@mtfishman
Copy link
Member Author

In the latest, I've merged ArrayStyle into DefaultArrayStyle, so now DefaultArrayStyle stores an extra type parameter for the array type. That seems to work well and is clearly a lot less code. I haven't seen any downsides yet, we'll see if there are any issues with downstream tests.

@mtfishman mtfishman changed the title Add ArrayInterface Add array type parameter to DefaultArrayInterface Jun 12, 2025
@mtfishman mtfishman merged commit 41f2ca2 into main Jun 12, 2025
19 checks passed
@mtfishman mtfishman deleted the mf/arrayinterface branch June 12, 2025 18:24
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