Skip to content

Better StructArray & StaticArray support#2546

Merged
avik-pal merged 39 commits intomainfrom
ptiede-structarrays
Apr 2, 2026
Merged

Better StructArray & StaticArray support#2546
avik-pal merged 39 commits intomainfrom
ptiede-structarrays

Conversation

@ptiede
Copy link
Copy Markdown
Collaborator

@ptiede ptiede commented Feb 25, 2026

No description provided.

@ptiede ptiede changed the title Draft to figure out better StructArray support Better StructArray support Feb 25, 2026
@ptiede ptiede changed the title Better StructArray support Better StructArray & StaticArray support Feb 26, 2026
@ptiede ptiede marked this pull request as ready for review February 26, 2026 05:53
else
typeof(res_tmp)
end
result = similar(first(flat_args), T_res, L)
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This should be reviewed closely. I had to change this line because now flat_args may have differing array types, e.g., one could be a StructArray. Before we decided the output entirely based on the first element, which could lead to errors, e.g., 2 .* sa would try to write a StructArray to a TracedRArray.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Oh this is clever!

@ptiede
Copy link
Copy Markdown
Collaborator Author

ptiede commented Feb 26, 2026

@wsmoses @jumerckx I think this is ready for a first review. I'll add some tests tomorrow, but this seems to enable more support for StructArrays. Right now, the only thing that could cause an issue is if one of the underlying arrays isn't a ReactantPrimitive, but that should be enough for what I need.

sbrantq added a commit that referenced this pull request Mar 12, 2026
ptiede and others added 3 commits March 23, 2026 18:46
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@ptiede
Copy link
Copy Markdown
Collaborator Author

ptiede commented Mar 24, 2026

@avik-pal @Pangoraw @wsmoses I think this is ready for review. Once this is merged, most of Comrade should work.

@ptiede
Copy link
Copy Markdown
Collaborator Author

ptiede commented Mar 24, 2026

Actually one question is what are the semantics of

overloaded_mul(A, B, alpha, beta)

typically for the 5-argument mul in place is

C = alpha A*B + beta * C

I'm not sure what this should be when C isn't defined.

) where {T,TI<:Integer}
valsT = maybe_convert_elt(T, vals)
foreachfield((col, val) -> (@inbounds col[I] = val), s, valsT)
foreachfield((col, val) -> (@inbounds Reactant.@allowscalar col[I] = val), s, valsT)
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.

lambda functions are going to cause some compile time issues, can we get rid of them with functions defined directly (possibly with fix1/etc as relevant)?

Copy link
Copy Markdown
Member

@wsmoses wsmoses left a comment

Choose a reason for hiding this comment

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

lgtm minus lambda nits

@avik-pal avik-pal merged commit feba0ca into main Apr 2, 2026
94 of 128 checks passed
@avik-pal avik-pal deleted the ptiede-structarrays branch April 2, 2026 04:16
Pangoraw pushed a commit that referenced this pull request Apr 3, 2026
* Draft to figure out better StructArray support

* Simplify and generalize structarray type conversion

* Start adding StaticArray support

* Add StaticArray support and tweak elem_apply_while_loop to select correct container type

* Revert tracing.jl

* Remove info debug

* Remove get_ith

* Add _copyto!

* format

* Fix broken test and add new tests

* format

* add StaticArrays

* Add LinearAlgebra

* Remove unused function

* Reuse the known destination for while loop if possible

* Update ext/ReactantStructArraysExt.jl

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* Proposed improved support for SArrays

* fix dumb mistake

* Add additional changes for StaticArrays

* Apply suggestions from code review

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* Cleanup

* Update

* Update

* Format

* Fix for code review

* Add comments

* So dumb

* Correct comment in overloaded_mul function

Fix comment typo in overloaded_mul function.

* Update to remove anonymous functions

* Update

* Add a complex test

* Update

---------

Co-authored-by: Billy Moses <wmoses@google.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Avik Pal <avikpal@mit.edu>
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.

4 participants