Skip to content
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

Fix mul overload #440

Merged
merged 9 commits into from
Dec 31, 2024
Merged

Fix mul overload #440

merged 9 commits into from
Dec 31, 2024

Conversation

wsmoses
Copy link
Member

@wsmoses wsmoses commented Dec 31, 2024

No description provided.

@wsmoses wsmoses requested a review from avik-pal December 31, 2024 02:06
wsmoses and others added 3 commits December 30, 2024 21:13
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@@ -130,14 +130,18 @@ for (cT, aT, bT) in (
if any(Base.Fix2(isa, TracedRArray) ∘ ancestor, (C, A, B))
TracedLinearAlgebra.overloaded_mul!(C, A, B, α, β)
else
LinearAlgebra._mul!(C, A, B, α, β)
LinearAlgebra.mul!(C, A, B, α, β)
Copy link
Collaborator

Choose a reason for hiding this comment

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

this will very likely trigger the illegal instruction #393

Copy link
Member Author

Choose a reason for hiding this comment

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

There was an absint behavior that we had a while ago but was accidentally removed in a refactor which I re added with the last cuda stuff.

I hope it means we won't hit it

Copy link
Collaborator

Choose a reason for hiding this comment

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

I checked it again in the stack PR, the crash persists

Copy link
Member Author

Choose a reason for hiding this comment

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

Well darn, okay yeah if CI yells I think we need to make a separate noinline function which is called in the override, which itself calls mul

Copy link
Collaborator

Choose a reason for hiding this comment

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

avik-pal and others added 2 commits December 31, 2024 12:17
* fix: handle aos for mul

* Update ext/ReactantArrayInterfaceExt.jl

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

* revert: incorrect aos_to_soa for C

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@wsmoses
Copy link
Member Author

wsmoses commented Dec 31, 2024

@avik-pal it looks like this is hitting some scalar index stuff?

@wsmoses
Copy link
Member Author

wsmoses commented Dec 31, 2024

Yeah we need to change aos to anytracedarray

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@wsmoses
Copy link
Member Author

wsmoses commented Dec 31, 2024

Okay @avik-pal tesfs pass so I’m going to merge

@wsmoses wsmoses merged commit 85d8ba4 into main Dec 31, 2024
29 of 39 checks passed
@wsmoses wsmoses deleted the mulover branch December 31, 2024 22:34
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.

2 participants