Skip to content

fix: map! and map #892

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Mar 13, 2025
Merged

fix: map! and map #892

merged 3 commits into from
Mar 13, 2025

Conversation

@avik-pal
Copy link
Collaborator

@avik-pal avik-pal commented Mar 12, 2025

#882 but without #881

@wsmoses wsmoses requested a review from giordano March 12, 2025 20:40
@avik-pal
Copy link
Collaborator Author

avik-pal commented Mar 12, 2025

I need to add some tests here

giordano added a commit to PRONTOLab/GB-25 that referenced this pull request Mar 12, 2025
@giordano
Test EnzymeAD/Reactant.jl#892
5b73109
@giordano
Copy link
Member

giordano commented Mar 12, 2025

This is still crashing PRONTOLab/GB-25#48

@giordano
Copy link
Member

giordano commented Mar 12, 2025

Simone posted a reproducer: #889 (comment)

@avik-pal
Copy link
Collaborator Author

avik-pal commented Mar 13, 2025

atleast now we know the real source of the issue

@avik-pal avik-pal changed the title fix: dispatches for GB fix: dispatches for GB [Don't Merge] Mar 13, 2025
@avik-pal avik-pal mentioned this pull request Mar 13, 2025
Merged
@avik-pal avik-pal marked this pull request as draft March 13, 2025 01:16
avik-pal pushed a commit to PRONTOLab/GB-25 that referenced this pull request Mar 13, 2025
@giordano @avik-pal
Test EnzymeAD/Reactant.jl#892
bbce471
@avik-pal
Copy link
Collaborator Author

avik-pal commented Mar 13, 2025

fixes #889

avik-pal
avik-pal commented Mar 13, 2025
View reviewed changes
src/ConcreteRArray.jl
return R
end

function mymap!(f, R, A)
Copy link
Collaborator Author

@avik-pal avik-pal Mar 13, 2025

Choose a reason for hiding this comment

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

same as the mymapreduce!, we are definitely doing some incorrect codegen for in place updates, else this change shouldn't matter

@avik-pal avik-pal changed the title fix: dispatches for GB [Don't Merge] fix: dispatches for GB Mar 13, 2025
@avik-pal avik-pal changed the title fix: dispatches for GB fix: map! and map Mar 13, 2025
@avik-pal avik-pal marked this pull request as ready for review March 13, 2025 03:05
@avik-pal
Copy link
Collaborator Author

avik-pal commented Mar 13, 2025

PRONTOLab/GB-25#48 is passing with the latest commit

@wsmoses
Copy link
Member

wsmoses commented Mar 13, 2025

mose is off for the night but if it works for you, go for it

@giordano giordano linked an issue Mar 13, 2025 that may be closed by this pull request
Reduction PR #881 breaks Oceananigans #889
Closed
@avik-pal avik-pal merged commit 74e8098 into main Mar 13, 2025
54 of 56 checks passed
@avik-pal avik-pal deleted the ap/gb-scalar3 branch March 13, 2025 03:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wsmoses wsmoses wsmoses approved these changes

@giordano giordano Awaiting requested review from giordano

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Reduction PR #881 breaks Oceananigans

4 participants

@avik-pal @giordano @wsmoses